Skip to content

cel: complete translator cutover and deadframe integration - #1320

Open
youknowone wants to merge 15 commits into
mainfrom
cel
Open

cel: complete translator cutover and deadframe integration#1320
youknowone wants to merge 15 commits into
mainfrom
cel

Conversation

@youknowone

Copy link
Copy Markdown
Owner

What

  • Complete the remaining CEL-oriented translator cutover, indirect-family census, and MIR corpus coverage.
  • Move libc-backed deadframe storage into the backend-neutral crate and verify GC-visible deadframe slots across dynasm and Cranelift.
  • Add CEL release gates for scalar, column, policy, float, and probe execution paths.
  • Record structured decline reasons for translation walls and keep the gate triage documents synchronized.

Why

This is the CEL-specific remainder after extracting the reusable majit work into the base branch. Keeping it as one commit directly above majit-runtime-followup makes the merge order explicit and allows a linear merge after the base PR lands.

Stack

  • Base: majit-runtime-followup (must land first)
  • This branch is exactly one commit above the base.

Checks

  • Fresh LLBC extraction for majit-rlib, pyre-object, pyre-interpreter, and pyre-jit
  • cargo check --features dynasm
  • cargo test --features dynasm
  • cargo test --all --no-default-features --features dynasm
  • cargo test --release -p cel --no-default-features --features dynasm (5/5)
  • python3 scripts/check-citation-drift.py --self-test
  • RUSTDOCFLAGS='-D warnings' cargo doc --no-deps --locked -p majit-rlib -p majit-trace
  • python3 pyre/check.py --backend dynasm --no-synthetic (dynasm 10/10; fib_recursive counters were unstable and non-gated on this run)

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 13 minutes.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 44c8ff0d-5d50-49ee-b3c6-e4826d5c84de

📥 Commits

Reviewing files that changed from the base of the PR and between 62d2029 and aa0afe9.

📒 Files selected for processing (51)
  • .github/workflows/pyre-ci.yml
  • AGENTS.md
  • majit/charon-corpus/README.md
  • majit/examples/cel/src/colscalar.rs
  • majit/examples/cel/src/column.rs
  • majit/examples/cel/src/float.rs
  • majit/examples/cel/src/main.rs
  • majit/examples/cel/src/policy.rs
  • majit/examples/cel/src/probe.rs
  • majit/examples/dualtape/src/jit_interp.rs
  • majit/examples/spcount/src/main.rs
  • majit/examples/tinyframe/src/jit_interp.rs
  • majit/examples/tla/src/jit_interp.rs
  • majit/examples/tlr/src/jit_interp.rs
  • majit/gate-triage.md
  • majit/majit-backend-cranelift/Cargo.toml
  • majit/majit-backend-cranelift/src/compiler.rs
  • majit/majit-backend/src/deadframe.rs
  • majit/majit-charon-reader/tests/corpus.rs
  • majit/majit-macros/src/jit_interp/jitcode_lower/lower_value.rs
  • majit/majit-macros/src/jit_interp/mod.rs
  • majit/majit-metainterp/Cargo.toml
  • majit/majit-metainterp/src/blackhole.rs
  • majit/majit-metainterp/src/embed.rs
  • majit/majit-metainterp/src/intrinsics.rs
  • majit/majit-metainterp/src/jitcode/assembler.rs
  • majit/majit-metainterp/src/jitdriver.rs
  • majit/majit-metainterp/src/lib.rs
  • majit/majit-metainterp/src/pyjitpl.rs
  • majit/majit-metainterp/src/pyjitpl/dispatch.rs
  • majit/majit-metainterp/src/resume_box_reader.rs
  • majit/majit-metainterp/src/trace_ctx.rs
  • majit/majit-metainterp/src/warmstate.rs
  • majit/majit-metainterp/tests/arraybase_vable_has_no_emitter.rs
  • majit/majit-translate/src/codewriter/call.rs
  • majit/majit-translate/src/codewriter/codewriter.rs
  • majit/majit-translate/src/codewriter/insns.rs
  • majit/majit-translate/src/codewriter/policy.rs
  • majit/majit-translate/src/decline.rs
  • majit/majit-translate/src/front/mod.rs
  • majit/majit-translate/src/front/result_exc.rs
  • majit/majit-translate/src/front/slice_get.rs
  • majit/majit-translate/src/lib.rs
  • majit/majit-translate/src/model.rs
  • majit/majit-translate/src/parse.rs
  • majit/majit-translate/src/translator/driver.rs
  • majit/majit-translate/src/translator/rtyper/cutover.rs
  • majit/majit-translate/tests/test_cel_census.rs
  • majit/majit-translate/tests/test_indirect_family_post_rtyper.rs
  • pyre/cpython_tests/run.py
  • pyre/gate-triage.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@youknowone
youknowone marked this pull request as ready for review August 18, 2026 10:47

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ed6855157

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pyre/cpython_tests/run.py
entry[backend] = status
entry.setdefault("provenance", {})[backend] = dict(cell_axis)
continue
if status == shared:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh provenance for unchanged baseline cells

When --update-baseline remeasures an existing cell and its status still matches the shared verdict, this branch exits without writing cell_axis. Consequently, almost every pre-existing baseline entry remains permanently provenance-free—or retains an obsolete axis—even after being freshly measured, so axis_drift_report continues reporting it as legacy/drifted. Update the shared cell's provenance here; the analogous unchanged-overlay case below also needs its provenance refreshed.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 608c7164fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let arms = assert_portal_installed(interp, expected.is_empty());
let degraded = degraded_dispatch_arm_names(interp);
assert!(
degraded == expected,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize both sides before comparing degraded-arm sets

When a caller pins two or more correct arms in portal/source order—for example, ["ROLL", "ALLOCATE"]degraded_dispatch_arm_names sorts the actual names alphabetically but this comparison leaves expected untouched, so the documented set assertion panics even though the sets are identical. Normalize the expected names before comparison; assert_degraded_dispatch_arm_causes has the same order-sensitive comparison and should be normalized as well.

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit aa0afe9).
Updated: 2026-08-24T09:19:49.706Z

Files in the reviewed diff
.github/workflows/pyre-ci.yml
AGENTS.md
majit/charon-corpus/README.md
majit/examples/cel/src/colscalar.rs
majit/examples/cel/src/column.rs
majit/examples/cel/src/float.rs
majit/examples/cel/src/main.rs
majit/examples/cel/src/policy.rs
majit/examples/cel/src/probe.rs
majit/examples/dualtape/src/jit_interp.rs
majit/examples/spcount/src/main.rs
majit/examples/tinyframe/src/jit_interp.rs
majit/examples/tla/src/jit_interp.rs
majit/examples/tlr/src/jit_interp.rs
majit/gate-triage.md
majit/majit-backend-cranelift/Cargo.toml
majit/majit-backend-cranelift/src/compiler.rs
majit/majit-backend/src/deadframe.rs
majit/majit-charon-reader/tests/corpus.rs
majit/majit-macros/src/jit_interp/jitcode_lower/lower_value.rs
majit/majit-macros/src/jit_interp/mod.rs
majit/majit-metainterp/Cargo.toml
majit/majit-metainterp/src/blackhole.rs
majit/majit-metainterp/src/embed.rs
majit/majit-metainterp/src/intrinsics.rs
majit/majit-metainterp/src/jitcode/assembler.rs
majit/majit-metainterp/src/jitdriver.rs
majit/majit-metainterp/src/lib.rs
majit/majit-metainterp/src/pyjitpl.rs
majit/majit-metainterp/src/pyjitpl/dispatch.rs
majit/majit-metainterp/src/resume_box_reader.rs
majit/majit-metainterp/src/trace_ctx.rs
majit/majit-metainterp/src/warmstate.rs
majit/majit-metainterp/tests/arraybase_vable_has_no_emitter.rs
majit/majit-translate/src/codewriter/call.rs
majit/majit-translate/src/codewriter/codewriter.rs
majit/majit-translate/src/codewriter/insns.rs
majit/majit-translate/src/codewriter/policy.rs
majit/majit-translate/src/decline.rs
majit/majit-translate/src/front/mod.rs
majit/majit-translate/src/front/result_exc.rs
majit/majit-translate/src/front/slice_get.rs
majit/majit-translate/src/lib.rs
majit/majit-translate/src/model.rs
majit/majit-translate/src/parse.rs
majit/majit-translate/src/translator/driver.rs
majit/majit-translate/src/translator/rtyper/cutover.rs
majit/majit-translate/tests/test_cel_census.rs
majit/majit-translate/tests/test_indirect_family_post_rtyper.rs
pyre/cpython_tests/run.py
pyre/gate-triage.md

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

None.

3. Pre-existing mismatches (already present before this patch)

  • majit/majit-macros/src/jit_interp/jitcode_lower/lower_value.rs:524 ↔ rpython/rlib/rawstorage.py:127 — intrinsic recognition keys only on the final path segment, so an unrelated whatever::majit_raw_load_i64(base, ea) is lowered as raw memory access; the new test explicitly pins this at lower_value.rs:2710. RPython specializes the specific registered raw_storage_getitem callable, not arbitrary same-named functions. The matcher predates this patch; this patch only adds coverage confirming it.

4. Structural adaptations

  • majit/majit-metainterp/src/intrinsics.rs:45 ↔ rpython/rlib/rawstorage.py:127 — Rust-side i64-address intrinsic bodies stand in for RPython’s typed external-registry raw-storage operations. This is a fundamental Rust/RPython interface adaptation; the resulting raw load/store operation matches the upstream lowering shape.

  • majit/majit-translate/src/codewriter/insns.rs:404 ↔ rpython/jit/metainterp/pyjitpl.py:1189arraybase_vable/rdd>i is a Rust-state-array escape mechanism with no RPython opcode counterpart. It is deliberately unreachable (no emitter), so it creates no current observable divergence.

  • majit/majit-backend/src/deadframe.rs:21 ↔ rpython/jit/backend/llsupport/llmodel.py:298 — the per-thread Rust heap-frame pool is a fallback when no GC JITFRAME type is registered, whereas PyPy allocates a GC-managed jitframe. The pool preserves frame ownership and zeroing; it is an implementation-language allocation adaptation, not a semantic change.

The census test's `JitDriverSpec` literal lists every field explicitly, so it
also lists `green_kinds` and `red_kinds`. This driver declares no greens and no
reds, so the vectors parallel to them are empty -- which is also the value that
leaves the codewriter's kind check disabled.

Assisted-by: Claude
The module doc was rewritten and the rewrite dropped the reason the structure
has to stay per-op: an earlier version of this machine fused raw_load+add+i++
into one opcode, the clean interpreter became a one-op memory-bandwidth-bound
loop with no dispatch left to eliminate, and the JIT read 0.62x. That figure
measured the kernel, not the float JIT.

`0.62x`, the kernel-versus-JIT attribution, and the `column` probe precedent
appear nowhere else in the tree, so nothing carried them after the rewrite.
What remained states that the opcodes are separate but not that they must be,
which is the half a future reader needs: fusing them back reproduces the 0.62x
and invites the same misreading.

The rewrite is kept. This adds a paragraph after it rather than reverting to
the older wording, and leaves the reworded program list alone -- the op counts
there already carry what the dropped `(memory-bound ref)` / `(compute-bound)`
annotations said.

Doc comments only. Not covered by the rustdoc ratchet, which lists majit-rlib
and majit-trace, and carries no rpython/pypy citation for check-citation-drift
to validate.

Assisted-by: Claude
`majit/examples/dualtape`'s shape-gate doc kept the claim that an op count
cannot distinguish body shapes, but not the values it collides on (`1` an
empty dispatch, `5` a segmented runaway) nor the record that the doc asserted
the count was sufficient until the shape gate refuted it.

The `pyre/gate-triage.md` half of this change is gone: it restored detail into
the timeout caution in §1b, and `a6684cd6641` retired that section.

Assisted-by: Claude
The documented command was `cargo test --all --features dynasm`. With default
features on, the workspace's example interpreters default to `cranelift` and
supply it to the one shared `majit-metainterp`, while `pyre-jit` compiles in its
dynasm registrations — so that command builds a binary that dispatches through
one backend and holds the other's hooks.

The stated justification, that without the flag `majit-metainterp` emits
`compile_error!`, describes a failure that configuration cannot produce: the
examples already supply a backend, so the no-backend arm is never reached.

Replaced with `--no-default-features --features dynasm` and a reason that
matches what the flags do.

Assisted-by: Claude
`scripts/check-new-line-citations.py` reads what a change adds and rejects
an `upstream.py:LINE` inside a `//` comment. Of the fourteen it reports for
this branch, eleven are lines already carrying that citation in the base
copy of the same file, which this branch only reindents; three were written
here.

Each of the three already named, or sits next to, a symbol the number was
redundant with: `rclass.py:162-165` quotes `OBJECT = GcStruct('object', ...`,
which occurs once in that file; `descroperation.py:706-712` is `binop_impl`,
now named because the quote beside it occurs twice; `policy.py:48-84` already
spelled `look_inside_graph`.

Assisted-by: Claude
…mbedders

Two things every interpreter built on this framework writes for itself.

`intrinsics` holds the untraced bodies of the raw-memory, bitcast and
unsigned-arithmetic intrinsics the `#[jit_interp]` lowerer rewrites while
tracing. The lowerer matches the LAST segment of the call expression's
path and never looks at where the function is defined, so an imported
name is recognized exactly as a local `fn` of the same name is. Two tests
in `lower_value.rs` say so: `an_imported_intrinsic_lowers_like_a_local_one`
lowers the bare and the fully-qualified spelling of a load, a store and an
unsigned compare and asserts the two are equal, and
`only_the_last_path_segment_selects_an_intrinsic` is its control -- without
it, "the qualified spelling lowered the same" would also hold of a lowerer
that ignored the path and matched on argument shape alone.

A spelling that failed to be recognized would not error. It would lower as
a residual call, which is a worse graph and not a broken one, so the
property needs a test rather than a reading.

`embed::Census` holds the counters an embedder tallies off the four driver
callbacks, installs itself onto a driver, and reads the abort-reason
counters as a DELTA. The delta is the reason the type exists: the counters
are process-global, so "since process start" answers a different question
from "during this run", and two overlapping consumers of a resetting read
each see a fraction of the truth. Its serializing lock is a process-global
`Mutex` and not a `thread_local!`, which would serialize nothing.

`examples/tinyframe` is ported onto both as the proof, losing 93 lines of
its own copies and gaining 43.

Neither module has a same-named upstream file, so both are declared in the
local-boundary list in `lib.rs` with the reason: RPython needs no
intrinsics module because it lowers `rawstorage.py`, `longlong2float.py`
and `rarithmetic.py` at rtype time from the one definition each already
has, and the census's upstream equivalent is split between `warmspot.py`'s
test harness and `jitprof.py`, neither of which is a surface an embedder
calls.

Assisted-by: Claude
`assert_no_degraded_dispatch_arms` asserts the degraded set is empty. A
machine with a known lowering gap has a non-empty set, so the assertion
fails on it from the first call and the twelve examples that filter
`degraded_dispatch_arms()` by hand all wrote their own check instead.

`embed::assert_degraded_dispatch_arms` takes the expected set, so a known
gap is expressible: a new name means an arm stopped lowering, a missing
name means the pinned gap closed. `assert_no_degraded_dispatch_arms` is
now its `&[]` case. `assert_degraded_dispatch_arm_causes` pins each arm's
refusal family and `assert_degraded_dispatch_arm_reason_contains` pins the
source it refuses on; `degraded_dispatch_arm_names`,
`degraded_dispatch_arm_causes` and `degraded_dispatch_arm_reason` read the
registry without asserting. Both assertions look up the arm census first,
so an uninstalled portal reports that rather than reading as a closed gap.

spcount, tla and tlr drop their hand-rolled compile counters, shape flags
and probe mutex for `embed::Census` and the new assertion.

Assisted-by: Claude
…xes back to it

`seed_bridge_virtualizable_boxes` rebuilds the virtualizable shadow out of a
guard's fail args and stopped there, on the stated premise that every
`[.. ; virt]` state field is `RustVec`-stored and so carved out of
`synchronize_virtualizable`'s write-back. A state field holding a pointer to a
`[length][payload…]` block is not carved out, and for one of those the live
state struct kept the values the compiled run was entered with while the shadow
held the values at the guard.

Port `rebuild_state_after_failure`'s trailing `synchronize_virtualizable()` for
a vinfo with no `vable_token`. Also correct the freeze premise in the
`writeback_virt_array_state_fields` comment, which named the same carve-out.

Assisted-by: Claude
…a terminal return

A guard-resume bridge whose walk reaches the interpreted function's return sets
`single_pass_finish` and reports `usize::MAX` as its position. The
`jit_merge_point!` expansion reads that flag and `break`s; the `can_enter_jit!`
expansion did not, and assigned the position to the loop's `pc`, which indexes
out of bounds in a dispatch loop that is not bottom-tested on its program
length.

Emit the same `break` at the back edge, and leave the flag standing in
`bridge_from_guard_resume_position` for the caller to consume.

Assisted-by: Claude
`seed_jitframe_pool_arm` now answers pooled when `MAJIT_JITFRAME_POOL` is
unset, and reads `0` as a request for the owned arm, so the variable selects
either arm rather than only switching the pool on. `set_jitframe_pool` is
unchanged and still overrides it in both directions.

Doc comments updated to name the shipped arm: `FrameHeapOwner`'s summary,
the `pooled` field, `FrameHeapOwner::new`, `set_jitframe_pool`, the allocation
site in `run_compiled_code_inner`, and `count_owned_frame_buf`, whose
thread-local access and `RefCell` borrow per allocation are now charged to the
probe arm.

Adds the `MAJIT_JITFRAME_POOL` entry to `majit/gate-triage.md`.

Assisted-by: Claude
…rrect what it is for

`do_compile` gated `ctx.set_disasm(true)` and a `[jit][disasm-body]` print on
`MAJIT_DUMP_CLIF` for the trace body only. Adds the symmetric pair for the
host-callable `trace_N_entry` wrapper: `set_disasm` on its context, a
`[jit][code-size] wrapper_bytes=` line from the `wrapper_code_bytes` already
computed there, and `[jit][disasm-entry]`.

Corrects the comment above `body_call_conv`. It stated that Tail "clobbers
AppleAarch64 callee-saves x19-x28/x29" and gave that as the reason the wrapper
exists. `get_regs_clobbered_by_call` answers `ALL_CLOBBERS` for `(Tail, true)`,
the exception path, which emits no exception tables here; an ordinary Tail call
takes `DEFAULT_AAPCS_CLOBBERS` and preserves x19-x28. The wrapper's reason is
the pinned register, which `enable_pinned_reg` makes non-callee-saved in
Cranelift while AAPCS treats it as callee-saved.

Records what the emitted wrapper measures on aarch64: 13 instructions, 52
bytes, 4 memory ops over x19 and the fp/lr frame record, no callee-save block
and no fp saves, noted as an upper bound rather than a measured cost.

Updates the `MAJIT_DUMP_CLIF` read-site count in `majit/gate-triage.md` from
two to four.

Assisted-by: Claude
…stages and a residual

Adds `back-edge-stage-probe`, off by default, under which
`JitDriver::back_edge_internal` carries run-time selectable amplification loops
for three stages of a warm entry: the gate (`entry_procedure_token` on the
uncarried shape, `get_compiled_meta().cloned()`, `driver_descriptor_for`,
`is_compatible`), marshal-in (`sync_before`, `extract_live_values_into`,
`live_values_match_descriptor`, `extend_compiled_live_values_into` and the
buffer clears), and marshal-out (`restore_values`, `sync_after`).

`execute_assembler_at_dispatch_key` is not amplified and is documented as the
residual: it runs the trace, so a second pass would execute it again from state
the first advanced. `get_latest_descr_arc` and `decode_exit_slots` are inside
it, so the deadframe decode is in that residual too. The one-shot moves left
out of the three stages — `resolve_cell_key`, the single-pass dispatch key
take, `take_entry_scratch`'s `mem::take`, `entry_scratch_out`, and
`drop(result)` — are named on the fields that exclude them.

`BackEdgeStageRepeats` packs four u16 counts into one `AtomicU64`, so the entry
path takes one relaxed load that every arm pays. `set_back_edge_stage_repeats`
is the only selector; there is no environment gate, since all-zero is the
shipping shape. `back_edge_stage_passes` reports amplified passes per stage.

The marshal-in loop runs ahead of the shipping refill and ends each pass with
the clears `take_entry_scratch` performs, so the buffers the real extraction
fills are in the state it would have found them in. Marshal-out runs before the
arm split and skips `restore_values` for a FINISH, as that arm does.

Assisted-by: Claude
Adds a `execute-stage-probe` cargo feature to majit-metainterp (forwarded
to majit-backend-cranelift) that subdivides the stage `back_edge_internal`
spends inside `execute_assembler_at_dispatch_key`.

Four sub-stages, measured two different ways:

- prologue (`compiled_loops` probe, meta clone, `prepare_compiled_run_io`)
  and deadframe decode (`get_latest_descr_arc`, the fail-descr reads,
  `decode_exit_slots`) are repeatable, so they are AMPLIFIED: each is
  re-run `repeats` extra times and the difference over `repeats` is the
  stage.  An empty barrier loop carrying the same `black_box` barrier
  subtracts the amplification's own cost.
- `execute_token_with_dispatch_key` runs the trace and consumes its
  arguments, so it is not repeatable.  It is SINGLE-SHOT: clocked once per
  entry into `CALL_SHOT_NS`/`CALL_SHOT_COUNT`, with
  `execute_stage_clock_floor_ns` measuring this box's `Instant` floor for
  the caller to subtract.
- The jitframe build and input writes happen inside
  `run_compiled_code_inner`, i.e. INSIDE the single-shot call, not beside
  it.  `set_frame_build_repeats` amplifies them against a scratch
  `FrameHeapOwner` on the non-`use_gc_alloc` arm.  It is an "of which"
  figure and is not a summand of the stage total.

`ExecuteStageRepeats` packs the four counts into one process-global
`AtomicU64`; every arm pays the same relaxed load, so it cancels out of
their difference.  All-zero is the shipping shape and no env var reads it,
so no gate-triage row is minted.  With the feature off the crates build
byte-identical.

Assisted-by: Claude
…the flags

The sibling-door yield scan an embedder runs ahead of a warm entry asks
`has_runnable_compiled_loop`, which is a conjunction over three state
sources. Two of them are fused in the public API: `has_compiled_loop` is
`entry_procedure_token(..).is_some()`, and `JitCell::get_procedure_token`
performs the `Weak::upgrade` AND reads `invalidated` inside one function.

So no arm built from the public API can separate what the refcount pair
costs from what the flag reads cost. `WarmState::probe_cell_token_upgrades`
is that separation: the cell lookup, the upgrade, and the drop of the `Arc`
it produced, with no flag read after it. Differenced against
`has_compiled_loop`, it leaves the flags.

Its answer is deliberately WEAKER than the shipping predicate's -- an
invalidated token still upgrades, so it returns true where the door decides
false. It is a cost probe and never a decision, and the doc says so at all
three levels.

Behind `yield-stage-probe`, off by default, no production caller. Wrappers
on `MetaInterp` and `JitDriver` carry the same gate, so a default build has
neither the reader nor its symbol.

Assisted-by: Claude
…ion diamond

`core::slice::<Impl>::get` is a foreign leaf whose body is Opaque in the
LLBC, so it residualizes as an unregistered callee and the rtyper Skips
every calling graph. `front::slice_get` rewrites the residual call into
`if i < len(slice) { Some(slice[i]) } else { None }`, the general case of
the `front::slice_first` diamond.

`recognize_slice_get_site` pins the scalar `SliceIndex` instantiation by
the index operand's declared `usize`: `get(0..2)` returns `Option<&[T]>`,
and an element read at the range's start would hand the consumer a `T`
where a `[T]` is expected. Every range form falls through and keeps its
residual call. The rewrite is fail-safe throughout — a structural
mismatch returns `Err` and leaves the call untouched.

The `front/mir.rs` plumbing (site vector, capture site,
`recognize_slice_get_site`, post-pass driver, reachability gate) is not
included; that file carries another change in progress. Until it lands
the pass is compiled and never invoked, which is what the three
dead-code warnings report.

Assisted-by: Claude

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa0afe90a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +45 to +46
pub fn majit_raw_load_i8(base: i64, ea: i64) -> i64 {
unsafe { core::ptr::read_unaligned(raw_addr(base, ea) as *const i8) as i64 }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Mark raw-pointer intrinsics unsafe

When any safe Rust caller supplies a null, dangling, or otherwise invalid address, this public function executes read_unaligned and causes undefined behavior; documentation cannot transfer that safety obligation from a safe API to its callers. The same issue affects every public raw-load and raw-store function in this module, so they should be unsafe and the macro lowerer should preserve calls through unsafe blocks rather than weakening the interpreter API to accommodate the tracer.

AGENTS.md reference: AGENTS.md:L12-L15

Useful? React with 👍 / 👎.

Comment on lines +110 to +113
pub(crate) fn rewire_slice_get_call_sites(
graph: &mut FunctionGraph,
sites: &[SliceGetSite],
) -> usize {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Connect the slice-get pass to production lowering

For every translated body that calls <[T]>::get, this rewrite is currently unreachable: a repository-wide search at this commit finds SliceGetSite, rewire_slice_get_call_sites, and the claimed recognize_slice_get_site only in this new file, with all invocations confined to its unit tests. Consequently production MIR lowering never records or rewrites a site, so the residual core::slice::get call remains and its caller continues to be skipped by the rtyper rather than receiving the advertised bounds-checked diamond.

AGENTS.md reference: AGENTS.md:L206-L209

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant