mnemonic_builders.odin bakes `global_index + 1` into Instruction.enc_hint so the
encoder can skip the O(forms) match scan. The index is GLOBAL, so any edit to
the encoding table that inserts or removes a form shifts every index after it
and leaves the builders naming another instruction's encoding -- and nothing
noticed: the encoder took the form, emitted its bytes and returned success.
It has happened twice. 6e17e7a2d left 2130 of 3671 builders wrong; 36af73834
regenerated both halves and cleared it; baae2636b (adding `in`/`out`) re-broke
37; 9ae9a9bf9 shifted an early mnemonic and broke 3393 of 3802 -- including
CALL, whose r/m64 builder then encoded 0F 8A (JPE) instead of FF /2, turning
every indirect call into a conditional jump. The symptom was a segfault in a
JIT'd program, arbitrarily far from the cause.
Regenerated: 3820 of 3820 builders now correct. Three guards so it cannot
return silently:
- the encoder checks, under ODIN_DEBUG, that the hinted form lies inside its
own mnemonic's ENCODE_RUNS entry. Release keeps the byte-for-byte fast path;
this is a regeneration-time mistake and only has to be caught once by
anyone running tests.
- the generator stamps BUILDER_TABLE_FINGERPRINT, an FNV-1a hash of every
ENCODE_RUNS (start, count), into the file it emits.
- run_builder_generation_test recomputes it from the loaded tables and fails
with the command to regenerate.
Known-failing at this commit: four SAL/SHL cases, a separate defect in the
alias table, fixed in the commit that follows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Riok9vMpkLmo78wsVKJHhz
Fix the operand-size/prefix bug class reported by the sigil backend, then close
every failure a whole-ISA encode->decode->re-encode audit surfaced -- 247 -> 0
over the 1165 round-trippable legacy forms.
Encode/decode:
- MOVSX/MOVZX take 66h from the destination, not the source (CRC32, whose r/m
source drives 66h, is unchanged)
- operand-less 16-bit forms (CBW/CWD, MOVSW/CMPSW/SCASW/LODSW/STOSW,
IRET/PUSHF/POPF) emit 66h via a new opsize_16 flag
- CRC32/MOVBE and POPCNT/LZCNT/TZCNT: mandatory F2/F3 beats 66h on decode
- mandatory-prefix REP double-encode: F2/F3 no longer re-emitted as REP
- fixed-ModR/M decode: x87 (FNOP/FCHS/FADD ST(i)/...) and the 0F 01/AE/C7/1E
system-op groups (VMCALL/RDTSCP/LFENCE/ENDBR/...); gen.odin preserves the
fixed ModR/M byte, the decoder matches and consumes it
- implicit-operand round-trip (accumulator short forms, x87 ST(i), xchg) via a
positional matcher + emit path; accumulators stay implicit so hand-built
`add eax, imm` keeps the general form (typed-builder consistency)
- MOV to/from CR/DR/segment (were decoding as GPRs); reg-vs-mem disambiguation
(RDRAND vs VMPTRLD, MOVLHPS vs MOVHPS); x87 M80 operand size; PINSRW; NOP vs
XCHG at 0x90
- LOCK standalone; ENTER two immediates; PUSH/POP FS/GS (segment fixed by the
opcode + non-cacheable); BOUND marked 32-bit-only; RDRAND/RDSEED r16
(66 is operand-size, resolved against the mandatory-66 VMCLEAR by ModR/M)
Tools/tests:
- revive dump_verify_input + verify_against_llvm (stale Result API; drop the
aliases that were masking the fixed bugs); align verify_tables with the
preserved fixed-ModR/M ext
- new tests/test_narrow_widths.odin: byte-exact vs llvm-mc + round-trip guards
- fold the bit-rotted tests32 package into the main suite (i386 Mode._32 paths)
248 tests pass; verify_tables passes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01115ktMoX5qGAfp1VemBzCP
Data-oriented pass on the encode hot path. Profiling showed bounds checks
already elided by -o:speed; the cost was per-instruction loop/scan machinery
and immediates falling off the hint path.
- Gather the immediate slot in the single resolve pass and emit it straight-
line (no scan over enc.enc); likewise drive the legacy REX prefix from the
precomputed reg/mr/opr slots instead of a per-form scan.
- Fold the separate needs_66 (GPR16) and SPL/BPL/SIL/DIL operand loops into
the resolve pass, so user operands are visited exactly once. This was the
big one: mov r,r 27 -> 21 ns.
- Gate the whole legacy-prefix block on a single flags!=0 test (a legacy
prefix is almost always absent) instead of four branches per instruction.
- Make immediate forms hintable. A typed immediate builder names its width
(inst_add_r32_imm32), the matcher already keys off the operand's declared
size, so baking the form is byte-identical AND drops immediates from the
full match scan: mov r32,imm32 55.7 -> 17.8 ns (3.1x).
Floor (no-op) 14.55 -> 10.3 ns; realistic immediate-heavy typed mix
30 -> 20.5 ns/inst (~49 M inst/s). gen/builders/check/test/idempotent green;
2282 cases (typed==generic byte-identical, incl. the new immediate cases).
Targeted branchless revert + the pre-matched form fast path, and a fix
for a pre-existing bug the latter surfaced.
(a) Revert the two speculative-write spots from the prior branchless pass
(legacy-prefix emission, widened displacement store, ENCODE_TAIL_SLACK)
back to predicted branches. In real streams a legacy prefix is almost
always absent and disp size is stable, so those branches are ~free and
the unconditional stores only added work. Every class got faster
(RET 19->17.5, MOV r,r 52->46.6, VADDPS 42.8->39.3 ns).
(b) Pre-matched form hint. Instruction.enc_hint (in the existing 11-byte
padding, idx+1 biased; 0 = matcher path) lets a typed builder that maps
to a single value-independent form bake the global form index, so
encode() skips the O(forms) match scan -- and, in a varied stream, its
unpredictable branches. Generated for non-immediate forms only (value-
dependent imm8/imm32 selection stays on the matcher). On a 100k mixed
typed-builder stream: 47.3 -> 30.2 ns/inst (-36%), byte-identical to the
matcher path -- ~2x the original baseline for codegen.
Repair the typed inst_/emit_ builders. They were non-functional: the
generator cast the hw-only typed enum straight to Register
(Register(GPR64.RAX) -> class 0), so every typed-builder operand was
rejected by the matcher (encode returned empty). Untested because the
suite builds via the generic constructors. Now they build through the
class-correct op_gpr64/op_xmm/... path (op_* already used by 3+ operand
builders), emit_ reuses inst_, and a new 30-case consistency suite
asserts typed == generic (llvm-verified) and hint == matcher.
gen/builders/check/test/idempotent all green; 2276 cases.
Move all ten ISA packages (x86, arm32, arm64, mips, riscv, ppc, ppc_vle,
rsp, mos6502, mos65816) from core/rexcode/<arch> to core/rexcode/isa/<arch>,
so the import pattern is now `import "core:rexcode/isa/x86"`. The shared
core stays at core:rexcode/isa.
Mechanical: relative `import "../isa"` / "../../isa" -> absolute
"core:rexcode/isa" (the only path that survives the move; the "../" and
"../.." self/generated imports move with their packages). build.lua now
builds paths as <root>/isa/<name>; stale `cd <arch>` hints in the verify
tools and the doc.odin paths updated.
WASM stays at core/rexcode/wasm for now -- it is an IR, not an ISA, and
will move under the forthcoming core:rexcode/ir once that layer lands.
All 10 arches gen/builders/check/test green; import core:rexcode/isa/x86
verified working; wasm still compiles.
