Repository navigation
jit: retire two default-on FBW inline gates; const-class carrier-raise handler fix - #759
Conversation
WalkthroughThe changes add bridge-guard exception storage to ChangesBridge exception-resume and inline-walk behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant drive_bridge_carrier_walk
participant full_body_walk_trace
participant dispatch_via_miframe
participant walk_position
drive_bridge_carrier_walk->>full_body_walk_trace: compile root after single-recipe SubRaise
full_body_walk_trace->>dispatch_via_miframe: resume carrier walk with raise seed
dispatch_via_miframe->>walk_position: consume carrier_raise_seed
walk_position->>walk_position: mark exception class as constant
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
3132c75 to
71806db
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3132c754a1
ℹ️ 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".
| // `catch_exception/L` for the enclosing try sits BEHIND it (between the | ||
| // CALL's post-call `-live-` and the next op), so scan backward — the same | ||
| // lookup the single-frame exception-edge router uses. | ||
| let candidate = crate::jitcode_dispatch::find_catch_before_resume_live(code, root_pc); |
There was a problem hiding this comment.
For a carrier resume, the root frame pc is the after-residual -live- marker stored in the parent snapshot, while the CALL's catch_exception/L is immediately after that marker (the existing inline-call SubRaise path probes forward with try_catch_exception_at(code, op.next_pc)). Scanning backward from root_pc therefore misses the root handler, so carrier-boundary raises inside these try-block callees record RaiseNoRootCatch and fall back/deopt on every hot raise; the default-on inline widening makes those shapes eligible but then unable to bridge. Probe forward from the live marker instead.
AGENTS.md reference: AGENTS.md:L194-L195
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
majit/majit-metainterp/src/trace_ctx.rs (1)
1342-1361: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDoc comment now misdescribes
set_bridge_guard_exc; it belongs onbridge_source_is_exception_guard.The comment "True only for bridge traces sourced from an exception guard descr." was attached to
bridge_source_is_exception_guard()before this change; insertingset_bridge_guard_exc/bridge_guard_exc()between the comment and the method left it misplaced, andbridge_source_is_exception_guard()now has no doc comment.📝 Proposed fix
- /// True only for bridge traces sourced from an exception guard descr. pub fn set_bridge_guard_exc(&mut self, guard_exc: i64) { self.bridge_guard_exc = guard_exc; } /// The exception value grabbed at the guard failure that triggered this /// bridge (0 when none). See `bridge_guard_exc`. pub fn bridge_guard_exc(&self) -> i64 { self.bridge_guard_exc } + /// True only for bridge traces sourced from an exception guard descr. pub fn bridge_source_is_exception_guard(&self) -> bool { self.bridge_source_is_exception_guard }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@majit/majit-metainterp/src/trace_ctx.rs` around lines 1342 - 1361, Move the “True only for bridge traces sourced from an exception guard descr.” doc comment from set_bridge_guard_exc to bridge_source_is_exception_guard, placing it immediately before that method. Add an appropriate doc comment for set_bridge_guard_exc or bridge_guard_exc describing the stored exception value, preserving the existing bridge_guard_exc semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/gate-triage.md`:
- Line 173: Update the §5 gate inventory in pyre/gate-triage.md to include
PYRE_CARRIER_EXC_RESUME alongside PYRE_FBW_BRIDGE_SUBWALK_BINOP_INLINE, marking
it as the new default-off carrier-resume gate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs`:
- Around line 380-393: Set the walker’s class-of-last-exception constant flag to
true in the carrier_raise_seed catch-entry branch before calling
vstack_enter_exception_handler, matching the sibling exc_edge_catch_target and
raise/r handler paths.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs`:
- Around line 1076-1112: Restructure the `fbw_carrier_raise_enabled()` path in
the `rejoins` calculation so it resolves only
`try_catch_exception_at(caller_jitcode, resume_pos)` and passes that target to
`exc_handler_rejoins_loop`, without requiring `enclosing_loop_header_jit_pc`.
Preserve the existing `.zip()` and `exc_handler_rejoins_specific_loop`
requirement for the carrier-disabled path, which still needs the call’s own loop
header.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs`:
- Around line 73-85: Add positive-path tests to
inline_caller_frame_distinguishes_try_block_catch_marker_decline, using real
jitcode with a handler that rejoins its loop. Cover both
fbw_carrier_raise_enabled() ANY-loop-rejoin admission and
exc_handler_rejoins_specific_loop admission, ensuring the assertions accept the
caller frame and exercise the relevant loop/catch metadata rather than empty
jitcode.
---
Outside diff comments:
In `@majit/majit-metainterp/src/trace_ctx.rs`:
- Around line 1342-1361: Move the “True only for bridge traces sourced from an
exception guard descr.” doc comment from set_bridge_guard_exc to
bridge_source_is_exception_guard, placing it immediately before that method. Add
an appropriate doc comment for set_bridge_guard_exc or bridge_guard_exc
describing the stored exception value, preserving the existing bridge_guard_exc
semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0d1f0be9-7cb5-481b-8f4a-40d60ec77317
📒 Files selected for processing (15)
majit/majit-metainterp/src/jitdriver.rsmajit/majit-metainterp/src/trace_ctx.rspyre/bench/bridge_branchy_callee_regression.pypyre/check.pypyre/gate-triage.mdpyre/pyre-interpreter/src/executioncontext.rspyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rspyre/pyre-jit-trace/src/jitcode_dispatch/tests.rspyre/pyre-jit-trace/src/state.rspyre/pyre-jit-trace/src/trace.rspyre/pyre-jit/src/call_jit.rs
💤 Files with no reviewable changes (1)
- pyre/pyre-interpreter/src/executioncontext.rs
| fn inline_caller_frame_distinguishes_try_block_catch_marker_decline() { | ||
| // No after-residual catch resume → not a try-block CALL → accept. | ||
| assert_eq!( | ||
| decline_inline_caller_frame_for_catch_marker(Some(42)), | ||
| decline_inline_caller_frame_for_catch_marker(None, &[], 0), | ||
| Ok(()), | ||
| ); | ||
| // A try-block CALL with no resolvable loop header / catch (empty jitcode) | ||
| // cannot prove its handler rejoins the CALL's loop → decline. | ||
| assert_eq!( | ||
| decline_inline_caller_frame_for_catch_marker(Some(42), &[], 0), | ||
| Err(InlineCallerFrameDecline::TryBlockCatchMarker), | ||
| ); | ||
| assert_eq!(decline_inline_caller_frame_for_catch_marker(None), Ok(())); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add positive-path coverage for the try-block rejoin admission.
Both new cases only test the decline path (no resume, or unresolvable loop/catch on an empty jitcode). Neither the fbw_carrier_raise_enabled() ANY-loop-rejoin branch nor the exc_handler_rejoins_specific_loop branch is exercised with a real jitcode where the handler does rejoin a loop — the case this rework exists to admit. Since both gating flags default on, this is the production path and currently has no regression coverage; a test here would also have caught the .zip() conservatism gap flagged in resume_snapshot.rs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs` around lines 73 - 85, Add
positive-path tests to
inline_caller_frame_distinguishes_try_block_catch_marker_decline, using real
jitcode with a handler that rejoins its loop. Cover both
fbw_carrier_raise_enabled() ANY-loop-rejoin admission and
exc_handler_rejoins_specific_loop admission, ensuring the assertions accept the
caller frame and exercise the relevant loop/catch metadata rather than empty
jitcode.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 7b219bb). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
71806db to
d153e51
Compare
Review dispositionsAddressed the automated reviews (Codex + CodeRabbit + CI parity review). Two new commits: Fixed
Not a bug (verified)
Deferred (inert / staged)
Structural adaptations (intended)
— commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d153e5126e
ℹ️ 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".
| /// #343/#126 depth-2 exception-resume slice is validated bit-exact. | ||
| pub fn carrier_exc_resume_enabled() -> bool { | ||
| static ENABLED: std::sync::OnceLock<bool> = std::sync::OnceLock::new(); | ||
| *ENABLED.get_or_init(|| std::env::var_os("PYRE_CARRIER_EXC_RESUME").is_some()) |
There was a problem hiding this comment.
When a user or test harness exports PYRE_CARRIER_EXC_RESUME=0 to keep this default-off carrier exception resume disabled, this is_some() check still returns true. That unexpectedly runs the new exception-guard seeding path in setup_bridge_sym despite the comment advertising =1 enablement and the gate being documented as validation-only; use a value check such as == "1" or != "0" so rollback configs don't turn the experimental path on.
Useful? React with 👍 / 👎.
d153e51 to
aef820e
Compare
youknowone
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pyre/pyre-jit-trace/src/trace.rs (1)
1434-1609: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRequire blackhole continuations to apply or abort without later replay.
drive_single_frame_blackholeresumes the frame throughresume_mainloop()/run(), so side-effecting bytecode can execute beforeapply_single_frame_blackhole_crn()is reached. If that reconciliation returnsfalse,try_adopt_single_frame_blackhole()returnsfalseand walks back to the legacy residual-call path, which does not know about the blackhole continuation’s concrete effects. Track this case as a no-replay/abort condition so any run that produced execution effects cannot be followed by bytecode replay.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/trace.rs` around lines 1434 - 1609, Update try_adopt_single_frame_blackhole and its caller to distinguish “not adopted” from “abort without replay” when drive_single_frame_blackhole has executed effects but apply_single_frame_blackhole_crn fails. Propagate a no-replay/abort result through the walk-back path so the legacy residual-call path is never entered after blackhole execution, while preserving normal fallback only when no execution effects occurred.pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs (1)
329-332: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMove
CarrierRaiseSeedout of TLS and onto the active walk owner.This is interpreter-owned exception-routing state tied to a specific multi-frame carrier walk, not reusable thread-local cache. Store it on
WalkSessionor aTraceCtx-scoped context and consume it there so its ownership, lifetime, and GC rooting stay trace-scoped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs` around lines 329 - 332, Replace the TLS-based carrier raise seed retrieval in the bridge sub-walk flow with state owned by the active WalkSession or TraceCtx-scoped walk context. Update the code that seeds and consumes CarrierRaiseSeed, including drive_bridge_carrier_walk and the current bridge sub-walk owner, so the seed is consumed exactly once from that walk-scoped state and remains GC-rooted for the walk lifetime; remove reliance on take_carrier_raise_seed TLS storage.Source: Coding guidelines
♻️ Duplicate comments (1)
pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs (1)
73-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing positive-path test for the try-block rejoin admission (unaddressed from prior review).
Both test cases only exercise the decline path. Neither the ANY-loop-rejoin admission nor a case where
exc_handler_rejoins_loopactually returnstrueis exercised, so the production-path behavior this PR cohort is centered on (widening try-block CALL inlining when the handler rejoins a loop) has no regression coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs` around lines 73 - 85, Extend inline_caller_frame_distinguishes_try_block_catch_marker_decline with a positive try-block CALL case whose resolvable catch handler makes exc_handler_rejoins_loop return true. Assert that decline_inline_caller_frame_for_catch_marker returns Ok(()) for this loop-rejoining handler, while preserving the existing no-resolvable-catch decline case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/gate-triage.md`:
- Around line 70-84: Synchronize gate lifecycle documentation: in
pyre/gate-triage.md lines 70-84, document only the verified subwalk/drain gates
as retired and retain PYRE_CARRIER_EXC_RESUME as default-off in the gate
inventory; in pyre/bench/bridge_branchy_callee_regression.py lines 16-22, remove
wording that presents the gates as active or explicitly mark it historical.
---
Outside diff comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs`:
- Around line 329-332: Replace the TLS-based carrier raise seed retrieval in the
bridge sub-walk flow with state owned by the active WalkSession or
TraceCtx-scoped walk context. Update the code that seeds and consumes
CarrierRaiseSeed, including drive_bridge_carrier_walk and the current bridge
sub-walk owner, so the seed is consumed exactly once from that walk-scoped state
and remains GC-rooted for the walk lifetime; remove reliance on
take_carrier_raise_seed TLS storage.
In `@pyre/pyre-jit-trace/src/trace.rs`:
- Around line 1434-1609: Update try_adopt_single_frame_blackhole and its caller
to distinguish “not adopted” from “abort without replay” when
drive_single_frame_blackhole has executed effects but
apply_single_frame_blackhole_crn fails. Propagate a no-replay/abort result
through the walk-back path so the legacy residual-call path is never entered
after blackhole execution, while preserving normal fallback only when no
execution effects occurred.
---
Duplicate comments:
In `@pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs`:
- Around line 73-85: Extend
inline_caller_frame_distinguishes_try_block_catch_marker_decline with a positive
try-block CALL case whose resolvable catch handler makes
exc_handler_rejoins_loop return true. Assert that
decline_inline_caller_frame_for_catch_marker returns Ok(()) for this
loop-rejoining handler, while preserving the existing no-resolvable-catch
decline case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b68487d-fde6-4aee-b701-25f88edf3caa
📒 Files selected for processing (11)
majit/majit-metainterp/src/trace_ctx.rspyre/bench/bridge_branchy_callee_regression.pypyre/check.pypyre/gate-triage.mdpyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rspyre/pyre-jit-trace/src/jitcode_dispatch/tests.rspyre/pyre-jit-trace/src/trace.rs
aef820e to
34ad94c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@majit/majit-metainterp/src/trace_ctx.rs`:
- Around line 532-539: Update TraceCtx::bridge_guard_exc so the exception
reference is kept through a TraceCtx-owned GC-rooted mechanism, or re-grabbed
from a valid rooted source immediately before consumption; remove the untraced
raw i64 storage. Ensure set_bridge_guard_exc and the corresponding seed/read
path preserve the reference across moving collections and retain interpreter/JIT
semantic parity when the gate is enabled.
In `@pyre/pyre-jit-trace/src/trace.rs`:
- Line 1274: Update the single-recipe path around the carrier recipe check to
stop sourcing CarrierRaiseSeed from TLS; store it on the active TraceCtx/bridge
context and drain it from that same context. Ensure the seed’s exception payload
is properly rooted for its lifetime, then make this path unconditional without
relying on thread-local state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da0f0e7c-3ba4-48c2-af27-593bedfa7840
📒 Files selected for processing (11)
majit/majit-metainterp/src/trace_ctx.rspyre/bench/bridge_branchy_callee_regression.pypyre/check.pypyre/gate-triage.mdpyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rspyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rspyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rspyre/pyre-jit-trace/src/jitcode_dispatch/tests.rspyre/pyre-jit-trace/src/trace.rs
💤 Files with no reviewable changes (1)
- pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs
| /// | ||
| /// NOT a traced GC root: it is stored unconditionally but only dereferenced | ||
| /// under the default-off `PYRE_CARRIER_EXC_RESUME` gate, so today no live | ||
| /// deref can outlive a moving collection. Before that gate is flipped on | ||
| /// this must become a real root (or be re-grabbed at read time), since a | ||
| /// collection between `set_bridge_guard_exc` and the seed read would leave | ||
| /// the raw integer stale — tracked as a pre-flip-on parity gap. | ||
| pub(crate) bridge_guard_exc: i64, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Root bridge_guard_exc before this can be enabled.
This stores a GC reference as an untraced i64; a moving collection between write and consume leaves a stale pointer. A default-off environment gate does not make an enableable JIT semantic divergence safe. Store it through a rooted TraceCtx-owned mechanism or re-grab it at consumption. As per coding guidelines, “The generated JIT must preserve the interpreter's semantics.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@majit/majit-metainterp/src/trace_ctx.rs` around lines 532 - 539, Update
TraceCtx::bridge_guard_exc so the exception reference is kept through a
TraceCtx-owned GC-rooted mechanism, or re-grabbed from a valid rooted source
immediately before consumption; remove the untraced raw i64 storage. Ensure
set_bridge_guard_exc and the corresponding seed/read path preserve the reference
across moving collections and retain interpreter/JIT semantic parity when the
gate is enabled.
Source: Coding guidelines
| }; | ||
| if let Some((exc, exc_concrete)) = subwalk_raise { | ||
| if carrier.recipes.len() == 1 && crate::jitcode_dispatch::fbw_carrier_raise_enabled() { | ||
| if carrier.recipes.len() == 1 { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Move CarrierRaiseSeed off TLS before making this path unconditional.
This seed belongs to the active bridge/TraceCtx, not the OS thread. TLS can leak or be consumed by a reentrant same-thread walk; keep it in TraceCtx and drain it from that same context, with its exception payload properly rooted. As per coding guidelines, “Do not use TLS for process-global or interpreter-owned runtime state.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/src/trace.rs` at line 1274, Update the single-recipe path
around the carrier recipe check to stop sourcing CarrierRaiseSeed from TLS;
store it on the active TraceCtx/bridge context and drain it from that same
context. Ensure the seed’s exception payload is properly rooted for its
lifetime, then make this path unconditional without relying on thread-local
state.
Source: Coding guidelines
34ad94c to
897699f
Compare
PYRE_FBW_TRYBLOCK_INLINE and PYRE_FBW_CARRIER_RAISE both defaulted on with no remaining reason to toggle off once verified, so make their behavior unconditional and drop the gates: - a try-block CALL whose except handler rejoins a loop always multi-frame-inlines; decline_inline_caller_frame_for_catch_marker now runs the any-loop rejoin test directly instead of branching between it and a specific-loop fallback. - the depth-2 carrier-boundary raise delivery in drive_bridge_carrier_walk is always active. Deletes the now-dead exc_handler_rejoins_specific_loop and enclosing_loop_header_jit_pc (only the carrier-raise-off branch used them) and the now-unused call_jit_pc parameter of decline_inline_caller_frame_for_catch_marker. PYRE_CARRIER_EXC_RESUME stays (default-off lever). exceptions / exception_args_virtual / break_except_live_local bit-exact with loops_aborted=0; bridge_branchy_callee regression passes; check.py dynasm 303/303, cranelift 303/303, wasm 300/300. Assisted-by: Claude
…ume keep Record in the PYRE_P2_DRAIN retirement note that the branchy inlined-callee continuation gap's plain-branch subcase is closed unconditionally (the carrier-resume sub-walk int-binop inline landed without a gate in #748), and its exception-unwind subcase is handled by the now-unconditional try-block callee inline + carrier-boundary raise delivery (PYRE_FBW_TRYBLOCK_INLINE / PYRE_FBW_CARRIER_RAISE retired). Add PYRE_CARRIER_EXC_RESUME to Judged KEEP (default-off; the bridge_guard_exc GC-rooting and the unconditional execute_ll_raised exception assign are the parity gaps to close before it is enabled by default). Assisted-by: Claude
The carrier-boundary raise handler-entry branch in dispatch_via_miframe seeded last_exc_value + the handler-entry operand stack but left class_of_last_exc_is_const at its stale pre-bridge value, unlike the sibling exc_edge_catch_target branch and the walk-level SubRaise routing (opimpl_raise), which set it true. The seed's exception is a const-class raise (the callee's inline RAISE recorded a NewWithVtable of a known class), so a reraise in the handler read the standing exception state from the stale flag. Set it true to match the siblings. Document the default-off PYRE_CARRIER_EXC_RESUME pre-flip-on parity gaps (gate-triage.md, trace_ctx.rs): the bridge_guard_exc GCREF-as-i64 is not a traced root and the seed's execute_ll_raised assign is guarded by is_null rather than unconditional; both are inert while the gate is off. reraise-in-handler repro bit-exact (875000) on both backends; check.py dynasm 301/301, cranelift 301/301, wasm 297/297. Assisted-by: Claude
897699f to
7b219bb
Compare
youknowone
changed the title
Reduced to the unique delta over
mainafter #723 (exc-edge try-block cluster) and #748 (gh#343 branchy plain-branch int-binop inline, landed unconditional) absorbed this branch's earlier work, and rebased onto currentmain(which includes #765 fixing the #763 build break).What this PR does
1. Retire two default-on FBW inline gates (
fd1a528)PYRE_FBW_TRYBLOCK_INLINEandPYRE_FBW_CARRIER_RAISEwere verified default-on with no remaining reason to toggle off, so their behavior is made unconditional and the gates dropped:decline_inline_caller_frame_for_catch_markernow runs the any-loop rejoin test directly instead of branching between it and a specific-loop fallback.drive_bridge_carrier_walkis always active.exc_handler_rejoins_specific_loop/enclosing_loop_header_jit_pcand the now-unusedcall_jit_pcparameter ofdecline_inline_caller_frame_for_catch_marker.PYRE_CARRIER_EXC_RESUMEstays (default-off lever).2. gate-triage note (
12ae744)The branchy inlined-callee continuation gap's plain-branch subcase is closed unconditionally by #748; its exception-unwind subcase is handled by the now-unconditional try-block callee inline + carrier-boundary raise delivery.
PYRE_CARRIER_EXC_RESUMErecorded as kept (default-off; itsbridge_guard_excGC-rooting and unconditionalexecute_ll_raisedassign are the parity gaps to close before enabling).3. const-class carrier-raise handler fix (
7b219bb)Set
class_of_last_exc_is_conston the carrier-raise handler entry. #723 shipped thecarrier_raise_seedbranch without setting it.Verification
python3 ./pyre/check.py: dynasm 303/303, cranelift 303/303, wasm 300/300.— commented by Claude