Skip to content

fix(optimizer): optimized-path block/br_if branch lands mid-instruction (#483, #242) - #497

Merged
avrabe merged 1 commit into
mainfrom
fix/483-optimized-block-brif-label
Jun 25, 2026
Merged

fix(optimizer): optimized-path block/br_if branch lands mid-instruction (#483, #242)#497
avrabe merged 1 commit into
mainfrom
fix/483-optimized-block-brif-label

Conversation

@avrabe

@avrabe avrabe commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What

On the optimized (non---relocatable) ARM path, a forward block + br_if lowered to a conditional branch whose target landed mid-instruction and did not skip the block — silently miscompiling any such function (#483). The --relocatable direct path was unaffected.

Two root causes:

  1. Label id mismatch. A br/br_if exiting a forward block targets the id pushed when the Block opened (the Block arm emits only a Nop, no label). But the matching End emitted its label with the End's own id, so the branch resolved against an id no label carried and was left as the unpatched offset-0 placeholder. Fix: End labels with the id of the block it closes (loop ends / function-end keep their own id).

  2. Byte-size estimate gap. The byte_offsets table driving branch displacement sized Strh/Strb/Ldrh/Ldrsh/Ldrb/Ldrsb as the default 2 bytes, but the optimized path always addresses memory off a high base register (ip/r11) → the encoder emits the 4-byte .w form. Once branches resolved (fix 1), the i32.store16 in the repro drifted the target by 2. Fix: size these as 4.

Validation

  • New CI-gated unicorn differential (block_brif_483_differential.py): runs the forward-block repro and a nested-block + unconditional br case through the optimized path, diffing linear memory vs wasmtime across both branch directions. Fails without the fix (init_branch(0) wrongly executes the guarded stores, writing {0:33, 12:44}).
  • Frozen byte gate stays bit-identical (direct path untouched); full workspace suite green; fmt + clippy clean.

Scope / follow-ups

  • The size estimator also lacks correct arms for other variable-width ops (Cmp/Cmn/Adds/Subs/Popcnt) that could drift a branch spanning them — not triggered by this repro; filing a table-completeness follow-up.
  • A heavier nested fixture surfaced a separate pre-existing spill+control-flow frame bug (a sub sp,#N frame not deallocated before the epilogue pop) — independent of this fix; filing separately.

🤖 Generated with Claude Code

@avrabe @claude
fix(optimizer): optimized path block/br_if branch target lands mid-in…
c66151a 
…struction (#483, #242)

On the optimized (non-relocatable) ARM path, a forward `block` + `br_if` lowered
to a conditional branch whose target landed in the MIDDLE of an instruction and
did not skip the block, silently miscompiling any such function (the
`--relocatable` direct path was unaffected). Two root causes:

1. **Label id mismatch.** A `br`/`br_if` exiting a forward `block` targets the id
   pushed when the `Block` opened (the `Block` arm emits only a `Nop`). But the
   matching `End` emitted its label with the End's OWN id, so the branch resolved
   against an id no label carried and was left as the unpatched offset-0
   placeholder. Fix: `End` now labels with the id of the block it closes (loop
   ends and the function-end keep their own id — a loop's target label is already
   at its start, and the function end is just the structural trailing label).

2. **Byte-size estimate gap.** The `byte_offsets` table that drives branch
   displacement sized `Strh`/`Strb`/`Ldrh`/`Ldrsh`/`Ldrb`/`Ldrsb` as the default
   2 bytes, but the optimized path always addresses memory off a high base
   register (`ip`/r11), so the encoder emits the 4-byte `.w` form. Once the
   branch resolved (fix 1), the i32.store16 in the repro drifted the target by 2.
   Fix: size these as 4 (the optimized path never emits the 16-bit T1 form).

Validation: new CI-gated unicorn differential (`block_brif_483_differential.py`)
runs the forward-block repro AND a nested-block + `br` case through the optimized
path and diffs linear memory vs wasmtime across both branch directions; fails
without the fix (init_branch(0) wrongly executes the guarded stores). Frozen byte
gate stays bit-identical (direct path untouched); full workspace suite green.

The size-estimator also lacks correct arms for other variable-width ops (Cmp/Cmn/
Adds/Subs/Popcnt) that could drift a branch spanning them — filed separately as a
table-completeness follow-up; not triggered by this repro.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@avrabe
avrabe merged commit 024be09 into main Jun 25, 2026
19 checks passed
@avrabe
avrabe deleted the fix/483-optimized-block-brif-label branch June 25, 2026 21:26
This was referenced Jun 25, 2026
Closed
Merged
avrabe added a commit that referenced this pull request Jun 25, 2026
@avrabe @claude
docs(vcr): record optimized-path callee-saved/block-brif fixes landed…
dc1d122 
… + the optimizer_bridge control-flow frontier (#242) (#501)

VCR-ORACLE-001's running log was stale: it ended with "#490 blocks the non-leaf
prologue lever / the lever stays unmerged until #490 lands". Both that fix and a
companion control-flow fix have since landed this arc — record the resolution and
the broader frontier the work surfaced, so the #242 roadmap reflects reality and
prioritizes the next gated step.

Appended (behavior-frozen, doc-only) to VCR-ORACLE-001:
- #490 LANDED (PR #495): optimized path now wraps a body that touches r4-r8 in
  push {r4-r8,lr}/pop {r4-r8,pc}, decided on the post-realloc body (no spurious
  push; >4-param fns decline to direct so stack-arg offsets are untouched), CI
  oracle callee_saved_490_differential.py. The non-leaf prologue lever it blocked
  is now unblocked.
- #483 LANDED (PR #497): forward block+br_if mid-instruction landing — End
  labelled with its own id not the closed block's, plus Strh/Strb/Ldrh/... sized
  2 vs the 4-byte .w the high-base optimized path emits; CI oracle
  block_brif_483_differential.py.
- The multi-modal control-flow sweep established these are one cluster of
  optimizer_bridge control-flow/ABI defects — the concrete measured evidence for
  VCR-SEL-001's "collapse the two/three-selector accretion" motivation — and the
  next gated frontier: #500 (br_if still unresolved for if/else + sequential
  blocks; seqblocks the minimal repro; Pattern-3 ruled out), #499 (spill frame
  not deallocated under control flow; #490's pop epilogue made the SP leak fatal,
  #215-class), #498 (byte-size estimator incomplete for Cmp/Cmn/Adds/Subs/Popcnt;
  structural mirror drift), #496 (register-exhausting folds silently miscompiled).
  All four are separate gated steps, never idle-tick increments; none in shipped
  firmware (gale ships --relocatable/direct).

rivet validate profile unchanged (49 errors / 75 warnings / 0 broken cross-refs,
all pre-existing xref/coverage gaps; NON_XREF=0); coverage exit 0.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@avrabe avrabe mentioned this pull request Jun 26, 2026
Closed
avrabe added a commit that referenced this pull request Jul 15, 2026
@avrabe @claude
fix(#740): B<cond>.W (T3) packed HALF the halfword offset — wide cond…
7cf114b 
…itional branches landed mid-shape

Root cause (NOT the direct selector's label resolution, which computes the
halfword displacement correctly): the Thumb-2 encoder's 32-bit B<cond>.W
(encoding T3) arm packed `halfword_offset >> 1` into imm6:imm11. Per ARMv7-M
the byte offset is SignExtend(S:J2:J1:imm6:imm11:'0') — the 20-bit field
S:J2:J1:imm6:imm11 IS the signed halfword offset — so every conditional
branch spanning > 254 bytes jumped to HALF its intended displacement.
gust_poll's loop-head empty-budget `br_if 1 (;@2;)` (the only wide
conditional in the function) landed after the first transition call:
spurious state+0x34 write + spurious calls on a round that must not touch
memory. Narrow (16-bit) B<cond> was unaffected, which is why all
short-range CF differentials (#483/#497/#500/#508/#509) stayed green.

Fix packs the halfword offset directly (mirroring the T4 unconditional arm)
and loud-declines offsets outside the signed 20-bit T3 range instead of
truncating. Bytes cross-checked against llvm (`bne.w #0x224` = f040 8112).

Evidence:
- scripts/repro/brif_outer_740_differential.py — minimal loop-inside-block
  shape with wide exits on the direct path, RED pre-fix (5/6), GREEN now;
  anti-vacuous (fails if `poll` stops containing a T3 conditional); CI-wired.
- gust_spill_fwd_390_differential.py zero-state rows: the anti-vacuous #740
  xfail fired (FIXD) and is PROMOTED to strict — return AND state window
  now match wasmtime in all three lever configs.
- encoder unit test test_encode_thumb_bcond_wide_t3_halfword_offset_740
  (forward/backward wide, narrow byte-identical, out-of-range loud Err).
- Frozen anchors 10/10 bit-identical (no wide conditionals in fixtures);
  block_brif_483 / br_table_507 / br_table_value_509 / cf_shapes_500 /
  control_step differentials all PASS.

Closes #740. Part of the #242 Track-C validation lane.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L
@avrabe avrabe mentioned this pull request Jul 15, 2026
Merged
avrabe added a commit that referenced this pull request Jul 15, 2026
@avrabe @claude
fix(#740): Thumb-2 B<cond>.W (T3) halved the branch offset — wide con…
bc3a2c5 
…ditional branches landed mid-shape (#748)

* wip: session-limit salvage snapshot (agent died mid-lane; resume from here)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(#740): B<cond>.W (T3) packed HALF the halfword offset — wide conditional branches landed mid-shape

Root cause (NOT the direct selector's label resolution, which computes the
halfword displacement correctly): the Thumb-2 encoder's 32-bit B<cond>.W
(encoding T3) arm packed `halfword_offset >> 1` into imm6:imm11. Per ARMv7-M
the byte offset is SignExtend(S:J2:J1:imm6:imm11:'0') — the 20-bit field
S:J2:J1:imm6:imm11 IS the signed halfword offset — so every conditional
branch spanning > 254 bytes jumped to HALF its intended displacement.
gust_poll's loop-head empty-budget `br_if 1 (;@2;)` (the only wide
conditional in the function) landed after the first transition call:
spurious state+0x34 write + spurious calls on a round that must not touch
memory. Narrow (16-bit) B<cond> was unaffected, which is why all
short-range CF differentials (#483/#497/#500/#508/#509) stayed green.

Fix packs the halfword offset directly (mirroring the T4 unconditional arm)
and loud-declines offsets outside the signed 20-bit T3 range instead of
truncating. Bytes cross-checked against llvm (`bne.w #0x224` = f040 8112).

Evidence:
- scripts/repro/brif_outer_740_differential.py — minimal loop-inside-block
  shape with wide exits on the direct path, RED pre-fix (5/6), GREEN now;
  anti-vacuous (fails if `poll` stops containing a T3 conditional); CI-wired.
- gust_spill_fwd_390_differential.py zero-state rows: the anti-vacuous #740
  xfail fired (FIXD) and is PROMOTED to strict — return AND state window
  now match wasmtime in all three lever configs.
- encoder unit test test_encode_thumb_bcond_wide_t3_halfword_offset_740
  (forward/backward wide, narrow byte-identical, out-of-range loud Err).
- Frozen anchors 10/10 bit-identical (no wide conditionals in fixtures);
  block_brif_483 / br_table_507 / br_table_value_509 / cf_shapes_500 /
  control_step differentials all PASS.

Closes #740. Part of the #242 Track-C validation lane.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@avrabe