perf(codegen): replace the try/catch optnone contagion with targeted setjmp volatile slots (#6385)

[proggeramlug]

Fixes the optnone contagion behind #6385: merely having a try in a function deoptimized the entire function, even when nothing ever threw.

The defect

Perry lowers try/catch to setjmp/longjmp (stmt/try_stmt.rs), not to LLVM unwind edges. longjmp restores the callee-saved registers and stack pointer that setjmp snapshotted, so any local LLVM parked in a register across the setjmp reverts to its setjmp-time value when the exception fires — the try body's mutations vanish in the catch. This is exactly C's setjmp hazard (C11 7.13.2.1p3).

codegen defended against it by stamping optnone on the whole function containing the try. That is correct (at -O0 every value is frame-resident, and the frame survives longjmp) but it is a sledgehammer: the loop counters, the arithmetic, the compares and the branches around the try all stopped being optimized too. Every try-containing function — and every direct async function, which gets the same setjmp-based rejection boundary — was compiled unoptimized.

The fix

Apply C's volatile rule precisely instead of disabling optimization wholesale. mem2reg/SROA refuse to promote an alloca that has any volatile load/store (isAllocaPromotable bails on isVolatile()), and volatile accesses can be neither elided nor reordered against each other — so a volatile-accessed slot provably lives in the frame across the setjmp, while everything else in the function optimizes normally.

• RegCounter (block.rs) gains a try-region depth counter. LlBlock::emit — the single choke point every emitter funnels through, including emit_raw — records the destination pointer of every store emitted while the depth is non-zero.
• lower_try brackets the try body and the finally-protected catch body with enter_try_region/exit_try_region; the async rejection boundary brackets the async body.
• LlFunction::to_ir then runs volatile_setjmp::apply_setjmp_volatile: it resolves those recorded pointers back to their base allocas (through getelementptr derivation, to a fixpoint) and rewrites every load/store of those allocas — function-wide — to volatile.
• attributes #1 drops optnone, keeping only noinline.

noinline stays: LLVM's isInlineViable already refuses to inline a function containing a returns_twice call, so it is belt-and-braces rather than load-bearing — but it keeps the setjmp frame's identity from depending on an internal inliner policy, at zero cost.

The volatile set, and why it is sound

The set is "every alloca this function stores into while inside a setjmp-protected region". One rule, no exceptions — that is what makes it auditable.

• It is a superset of the C condition. C says: an automatic object modified between the setjmp and the longjmp and read afterwards is indeterminate unless volatile. We drop the "and read afterwards" half. Marking a slot that the try writes but nothing reads afterwards is merely conservative, never wrong.
• The region is tracked by emission depth, not block index, so it automatically covers nested blocks, loops, nested trys, and the duplicated finally bodies — whatever is lowered while the region is open belongs to it, regardless of which basic block the instruction lands in. Nesting composes for free.
• Every access to a marked alloca is upgraded, not just the ones inside the try, because promotability is a property of the alloca — and because a plain load in the catch could otherwise be GVN-forwarded from a plain store that dominates the setjmp.
• Conversely, an alloca this function never stores to inside a try region cannot have been modified between the setjmp and the longjmp by this frame. The only other way to modify it is for its address to escape to a callee — and an alloca with a non-load/store user already defeats mem2reg/SROA, so those stay in memory anyway.
• Non-memory needs no help. An SSA value defined inside the try body cannot be read from the catch (it does not dominate it). An SSA value defined before the setjmp and used after is live across the call, so the register allocator either spills it to the frame (which longjmp preserves) or parks it in a callee-saved register (which longjmp restores to the same, unmodified value).
• Module globals need no help. They are memory, and every try region is bracketed by opaque runtime calls (js_try_push, _setjmp, js_try_end, js_throw) that LLVM must assume clobber them, so they are never cached in a register across the boundary.
• Heap needs no help (objects, arrays, boxed closure captures, generator/async state objects, shadow-stack GC roots — the last go through js_shadow_slot_set, a runtime call, not an alloca).

On the #6385 benchmark this yields the intended set exactly. throughput() lowers to %r3 = acc, %r5 = i:

; before: define double @...throughput() #1 {   ; #1 = { noinline optnone }  ← whole fn dead
; after:  define double @...throughput() #1 {   ; #1 = { noinline }
  store volatile double 0.0, ptr %r3      ; acc  — written in the try body
  store          double 0.0, ptr %r5      ; i    — written only in for.update, OUTSIDE the try

acc stays frame-resident; the loop counter, the i & 1, the compare, the branch — and three redundant reloads of i that optnone used to preserve — all optimize normally.

Proving the volatile set is load-bearing

A correctness test that passes both with and without the fix proves nothing. PERRY_SETJMP_VOLATILE=0 (new, documented bisection-only switch, same spirit as PERRY_WRITE_BARRIERS=0) disables the promotion while still dropping optnone — i.e. it reproduces the miscompile the promotion exists to prevent.

test-files/test_gap_try_setjmp_volatile.ts is a 20-case adversarial suite (written before the optimization): written-in-try/read-in-catch, read-in-finally, read-after-the-try, let vs var, nested try, written-in-a-loop-in-the-try, written-in-a-closure, written-in-try-and-catch, heap object/array mutation, throw-before-vs-after the write, destructuring, try/finally with no catch, catch-param reassignment, async, generator.

Built at release (-O2/-O3; perry-dev at opt-level=1 hides the bug entirely):

	vs node --experimental-strip-types
promotion on (shipped default)	20/20 identical
PERRY_SETJMP_VOLATILE=0	13/20 silently wrong

The failures are exactly the predicted stale-value pattern — the try's write evaporates and the catch reads the pre-setjmp value:

 c1=42   → c1=1      (try does acc=41; catch reads acc==0, does acc+=1)
 c2=114  → c2=100    c3=6    → c3=1     c4=200  → c4=199
 c5=1055 → c5=1000   c6=15   → c6=8     c7=15   → c7=5
 c11=12,2,34,4 → c11=3,2,7,4            c12=113 → c12=100
 c13=111 → c13=100   c16=12,22 → c16=1,0          c20=1325 → c20=1000

Two of those earn their keep specifically: c12 (try/finally with no catch, whose finally re-runs on the exception path) and c13 (a write in the catch body of a try that has a finally — the catch body is itself setjmp-protected so the finally can re-run on a catch-body throw). Both regions are ones this PR deliberately brackets; both go red without them.

Before / after (release, macOS arm64, min-of-5×5, interleaved A/B)

shape	node	main	this PR
no try at all	3 ms	3 ms	3 ms
try/catch present, never throws	3 ms	16 ms	13 ms
throws 500k times (#6385's benchmark)	935 ms	178 ms	180 ms
JSON parse loop w/ throw bail-out (50k rows)	35 ms	38 ms	39 ms
try around a 20M-iteration hot loop	24 ms	79 ms	77 ms

Correctness is unchanged everywhere; the throwing benchmark does not regress (Perry already beats V8 there — the remaining gap to Hermes is structural, see below).

Honest reporting: this does NOT close most of the never-throws gap

The issue's framing (and my initial hypothesis) was that the 3 ms → 16 ms penalty for having a try was mostly optnone. It is not. Removing optnone buys ~3 of the 13 ms. The other ~10 ms is the js_try_push + _setjmp + js_try_end call sequence executed once per loop iteration — ~10 ns × 1M. That is structural to setjmp-based EH and is not addressable by this PR; closing it means a zero-cost-until-thrown table-driven scheme, which is a much larger change. I'd rather say so than dress up a 1.25x as a 5x.

What this PR does buy is the removal of a silent, general tax: every function containing a try — and every direct async function — was previously compiled with optimization off, whether or not the try was hot. That penalty scales with the size of the function, not with how often it throws, and it is now gone.

One measured regression, and why it is pre-existing

A synthetic "small try guard, then a hot loop" shape (bench_guard) went 79 ms → 93 ms. It is not the volatile set — the IR shows only flag marked volatile, with the loop counter free. The cause: for that exact function shape, Perry's -O3 output is slower than its -O0 output, and optnone was accidentally protecting it. The proof is that the identical loop with no try at all — same code path in both compilers, no volatile, no setjmp — runs at 123 ms, i.e. worse than either. So the -O3 cliff is pre-existing and reproducible without any try; this PR merely stops masking it for try-containing functions, and in fact lands better than the try-less baseline (93 ms vs 123 ms). Worth a separate issue; not a reason to keep optnone.

Validation

• cargo test -p perry-codegen --lib — 183 passed, including 7 new volatile_setjmp unit tests (store-destination parsing vs the stored pointer, GEP-derived stores marking the whole alloca, globals/heap left alone, already-volatile left alone, !invariant.load dropped on upgrade, no-op when there are no try stores).
  (The 2 pre-existing integration-test compile errors in perry-codegen — CompileOptions has no field named namespace_reexport_named_imports — are on main too and unrelated.)
• Adversarial suite at release: 20/20 vs node, and provably red without the promotion.
• Gap suite A/B vs a pristine origin/main toolchain: see comment below.
• cargo fmt --all -- --check clean; bash scripts/check_file_size.sh clean.

Files

• crates/perry-codegen/src/volatile_setjmp.rs (new) — the promotion pass + soundness argument + unit tests
• crates/perry-codegen/src/block.rs — RegCounter try-region depth + store recording in emit
• crates/perry-codegen/src/function.rs — enter/exit_try_region, to_ir runs the pass last
• crates/perry-codegen/src/stmt/try_stmt.rs — bracket the try body + finally-protected catch body
• crates/perry-codegen/src/stmt/mod.rs — bracket the async rejection boundary
• crates/perry-codegen/src/module.rs — attributes #1 = { noinline } (was { noinline optnone })
• test-files/test_gap_try_setjmp_volatile.ts (new) — the 20-case adversarial suite

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of try/catch/finally behavior across non-local control flow.
  • Prevented stale local values after exceptions, including nested, async, generator, closure, looping, and destructuring scenarios.
  • Preserved correct behavior when return handling is combined with exception paths.

• Tests

  • Added comprehensive coverage for setjmp/longjmp-related control-flow and local-variable behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

The codegen now tracks stores within setjmp-protected regions, promotes accesses to affected allocas to volatile, applies the transformation after return rewriting, removes function-wide optnone, and adds unit and adversarial control-flow coverage.

Changes

Setjmp volatile promotion

Layer / File(s)	Summary
Protected-region store tracking crates/perry-codegen/src/block.rs, crates/perry-codegen/src/function.rs, crates/perry-codegen/src/stmt/*	Nested protected regions record emitted store destinations, while try and async statement lowering brackets the relevant bodies.
LLVM volatile transformation crates/perry-codegen/src/volatile_setjmp.rs	LLVM IR parsing identifies affected allocas and upgrades their loads and stores to volatile, with focused unit coverage and an environment-controlled opt-out.
IR pipeline integration crates/perry-codegen/src/function.rs, crates/perry-codegen/src/lib.rs, crates/perry-codegen/src/module.rs	The transformation runs after return-site rewrites, the module is registered, and setjmp-related attributes no longer emit optnone.
Setjmp volatile behavior coverage test-files/test_gap_try_setjmp_volatile.ts	Adds cases for nested exception flow, local and heap mutations, destructuring, async rejection, generators, and loops.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the core change: replacing function-wide optnone with targeted setjmp volatile-slot handling.
Description check	✅ Passed	The description is detailed and covers summary, changes, issue reference, validation, and test evidence, even if it doesn't mirror the template headings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/6385-try-optnone-contagion

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-codegen/src/volatile_setjmp.rs (1)

127-139: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Pointer-provenance resolution only follows getelementptr, not phi/select.

derived_ptr resolves a derived pointer back to its base alloca only for getelementptr. If codegen ever produces a phi/select merging two alloca pointers (e.g. a pointer chosen depending on a branch) and only the load side goes through that merged pointer, this pass wouldn't recognize the connection to mark it volatile. In the common case this is masked because the direct store to the alloca (which the try region does capture) already blocks promotion of the whole alloca regardless of other unmarked accesses — but if a write itself ever reaches an alloca only through a phi/select pointer (never a direct store <ty> ..., ptr %alloca), try_region_stores would record the merged register instead of the alloca, and this pass would silently fail to protect it.

I don't see current codegen constructing phi/select of two alloca pointers for JS locals (locals are always accessed via a fixed slot or a getelementptr), so this is likely dormant risk rather than an active bug. Worth a defensive extension of derived_ptr to also resolve through phi/select operands, or a code comment flagging the limitation explicitly, so a future codegen change doesn't quietly reintroduce the exact class of bug this module fixes.

🛡️ Sketch of extending pointer resolution to phi/select

fn derived_ptr_operands(t: &str) -> Option<(&str, Vec<&str>)> {
    // existing getelementptr handling …
    if let Some(at) = t.find(" = phi ptr ") {
        let res = &t[..at];
        if !res.starts_with('%') { return None; }
        let rest = &t[at + " = phi ptr ".len()..];
        // parse "[ %a, %bb1 ], [ %b, %bb2 ]" and collect %a, %b …
    }
    None
}

Also applies to: 173-193

🤖 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 `@crates/perry-codegen/src/volatile_setjmp.rs` around lines 127 - 139, Extend
pointer-provenance resolution around derived_ptr and its callers to recognize
phi/select pointer results, recursively resolving each incoming pointer to the
underlying alloca before recording volatile stores or loads. Preserve the
existing getelementptr handling and ensure merged operands are parsed without
treating basic-block labels as pointers; if the current return shape cannot
represent multiple operands, update the surrounding resolution logic
accordingly.

🤖 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.

Nitpick comments:
In `@crates/perry-codegen/src/volatile_setjmp.rs`:
- Around line 127-139: Extend pointer-provenance resolution around derived_ptr
and its callers to recognize phi/select pointer results, recursively resolving
each incoming pointer to the underlying alloca before recording volatile stores
or loads. Preserve the existing getelementptr handling and ensure merged
operands are parsed without treating basic-block labels as pointers; if the
current return shape cannot represent multiple operands, update the surrounding
resolution logic accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0576722c-9ad2-461e-98dd-731b66b89005

📥 Commits

Reviewing files that changed from the base of the PR and between f0b676f and 5ac08c5.

📒 Files selected for processing (8)

• crates/perry-codegen/src/block.rs
• crates/perry-codegen/src/function.rs
• crates/perry-codegen/src/lib.rs
• crates/perry-codegen/src/module.rs
• crates/perry-codegen/src/stmt/mod.rs
• crates/perry-codegen/src/stmt/try_stmt.rs
• crates/perry-codegen/src/volatile_setjmp.rs
• test-files/test_gap_try_setjmp_volatile.ts

[proggeramlug]

Gap-suite A/B vs pristine origin/main — zero regressions

Full gap suite (319 tests), release build, macOS arm64, PERRY_NO_AUTO_OPTIMIZE=1:

Parity Pass:   297
Parity Fail:   18
Compile Fail:  1
Crashed:       1
Parity Rate:   93.9%

20 failures. 15 are already in test-parity/known_failures.json. The other 5 are not skip-listed, so I ran each one directly against a baseline compiler built from pristine origin/main (same runtime archives, same flags) and diffed the two outputs byte-for-byte:

not-skip-listed failure	baseline origin/main	this PR	verdict
test_gap_events_import_4995	fails vs node	byte-identical to baseline	pre-existing, untriaged on main
test_gap_handle_band_object_ops	fails vs node	byte-identical to baseline	pre-existing, untriaged on main
test_gap_http2_settings	fails vs node	byte-identical to baseline	pre-existing, untriaged on main
test_gap_http_overloads_3226plus	times out (exit 124)	times out (exit 124), byte-identical	pre-existing, untriaged on main
test_gap_node_fs	passes standalone	passes standalone, byte-identical	known flake under the suite's parallel/timing load; not a parity failure

So every gap failure on this branch either is already triaged, or reproduces identically on origin/main. Nothing this PR touches regressed.

try/catch-specific coverage

The tests most exposed to this change all pass: test_gap_try_finally_no_catch_rethrow, the async/generator families (the async transform shares the setjmp boundary this PR re-brackets), and the error-handling tests. The new test_gap_try_setjmp_volatile (20 adversarial cases) passes and — crucially — goes 13/20 red under PERRY_SETJMP_VOLATILE=0, which is what makes it evidence rather than decoration.