fix(encoder): CMP/ADDS/SUBS immediates via ThumbExpandImm — close the silent-miscompile class (#255)#256
Merged
Conversation
… silent-miscompile class (#255) gale #255: encode_thumb32_cmp_imm / _adds / _subs packed the raw `i:imm3:imm8` field with no ThumbExpandImm expansion — correct only for imm <= 0xFF, silently mis-encoding any larger value (e.g. `cmp #N` comparing the wrong constant → wrong branch). Same class as #251 (ORR/EOR NOP) and #253 (ADD/SUB frame). Fix (gale's suggested shared helper): add `try_thumb_expand_imm(u32) -> Option<u32>` — the correct ARMv7-M ThumbExpandImm reverse-encoder (zero-extended byte, the three replicated-byte patterns, and the 8-bit-rotated form). CMP/ADDS/SUBS now encode any valid modified immediate CORRECTLY and return Err on a genuinely unrepresentable one (CMP/S-forms have no plain-imm12 fallback), forcing the selector to materialize into a register rather than silently mis-compile. Note: gale's example `cmp #1000` is actually representable (1000 = 0xFA ror 30, field 0xF7A) — the old code mis-encoded it (raw 0x3E8); this encodes it right. The genuine error case is e.g. `#0x101` (bits too far apart for an 8-bit window). GATE: clippy clean; backend suite passes; three frozen differentials RESULT- identical (control_step 0x00210A55, flight_seam 0x07FDF307, div_const 338/338) — the fixed paths weren't exercised by fixtures (the comparison lowering uses reg-reg CMP), so this is purely defensive hardening of a latent class. Tests try_thumb_expand_imm_encodes_modified_immediates + cmp_adds_subs_immediate_error_on_non_modified_imm. Closes #255. Part of #242. (A follow-up can retrofit AND/ORR/EOR to share this helper too, extending their fold range beyond 0xFF.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #255 (gale).
The bug
encode_thumb32_cmp_imm/_adds/_subspacked the rawi:imm3:imm8field with no ThumbExpandImm expansion — correct only forimm ≤ 0xFF, silently mis-encoding larger values (acmpcomparing the wrong constant → wrong branch). Same class as #251 (ORR/EOR→NOP) and #253 (ADD/SUB frame).The fix (gale's suggested shared helper)
try_thumb_expand_imm(u32) -> Option<u32>— the correct ARMv7-M ThumbExpandImm reverse-encoder (zero-extended byte; the three replicated-byte patterns; the 8-bit-rotated form). CMP/ADDS/SUBS now:Note: gale's example
cmp #1000is actually representable (1000 = 0xFA ror 30, field0xF7A) — the old code mis-encoded it (raw0x3E8); this encodes it right. The genuine error case is e.g.#0x101(bits too far apart for an 8-bit window).Gate
clippy clean; backend suite passes; three frozen differentials result-identical (control_step
0x00210A55, flight_seam0x07FDF307, div_const338/338) — the fixed paths weren't exercised by fixtures (comparison lowering uses reg-regCMP), so this is defensive hardening of a latent class. Tests cover the helper + the three encoders' error path.Part of #242. Follow-up: retrofit AND/ORR/EOR to share
try_thumb_expand_immtoo (extends their fold range past0xFF).🤖 Generated with Claude Code