x86: the decoder's choice of mnemonic is declared, not emergent

Two defects, one shape: something the decoder must decide was never stated, so
it fell out of where an entry landed in an unstably sorted table.

CANONICITY. Several mnemonics name one encoding -- SHL and SAL are both /4, JE
and JZ are both 0x74. Decoding a representative of each and diffing against
llvm-mc: 74 patterns, 22 agreed, 52 did not (0F 84 -> JZ where llvm-mc says JE,
A4 -> MOVS vs MOVSB, DB E2 -> FCLEX vs FNCLEX). aliases.odin now declares 57
{alias, canonical} rows, each canonical name MEASURED from llvm-mc rather than
picked, and gen.odin drops an aliased entry as it collects decode entries -- so
the name never reaches the tables. The drop is conditional on the canonical name
covering the byte-identical encoding, which is what lets MOV be the alias at the
A0-A3/B8 moffs forms while staying the only name for 88/89. Aliases stay fully
encodable; only decoding narrows. 119 of 1350 legacy entries dropped, 74 of 74
now agree.

This also fixes the four SAL/SHL failures left by the previous commit, at their
root rather than by extending a hand-written list.

ADDRESS SIZE. 0xE3's mnemonic is chosen by which counter register it tests --
the 67h axis -- and REX.W does not affect address size at all. Modelling JRCXZ
as force_rex_w made its encoding 48 E3 cb where llvm-mc emits a bare E3 cb, gave
JECXZ the bare encoding that is really JRCXZ in long mode, and left three
indistinguishable entries. Encoding_Flags gains addr_size (2 of 6 spare bits);
the matcher refuses a form the mode cannot express, the encoder emits 67h when
it differs from the default, the decoder selects on it before the operand-size
pass. All ten cases match llvm-mc, refusals included.

The tests keep no second copy of the alias table: mnemonics_eq compares
canonical_mnemonic, and run_alias_table_test asserts the stronger property the
generator guarantees -- zero encodings spelled by two surviving mnemonics. It
caught the E3 ambiguity on its own.

Also fixed: gen.odin's write_flags enumerates Encoding_Flags by hand and
silently dropped addr_size from the generated tables -- the flag read back as
its zero value and the instruction was quietly mis-modelled.

271 passed. Bites: dropping {.SAL, .SHL} returns the prime_sieve failure and the
guard names every shift-group opcode; restoring force_rex_w decodes E3 00 as
JCXZ and encodes JRCXZ to three bytes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Riok9vMpkLmo78wsVKJHhz

core/rexcode/isa/x86/aliases.odin

@@ -0,0 +1,130 @@
+// rexcode  ·  Brendan Punsky (dotbmp@github), original author
+
+package rexcode_x86
+
+// =============================================================================
+// MNEMONIC ALIASES  —  which of several names for one encoding a disassembler prints
+// =============================================================================
+//
+// Some x86 instructions have several legal mnemonics for ONE encoding. SHL and
+// SAL are both ModRM.reg=4 in the shift group; JE and JZ are both 0x74; there
+// are 57 such pairs. A decoder has nothing to tell them apart with — the bytes
+// are identical — so it must simply pick a name, and that choice has to be
+// DECLARED. Left undeclared it falls out of wherever the entry happened to land
+// in the (unstably sorted) decode table: correct, arbitrary, and free to move on
+// any table regeneration. It did move once, and four tests that had always
+// passed began reporting `SAL != expected SHL`.
+//
+// The canonical name here is the one **llvm-mc prints**, measured rather than
+// chosen: llvm-mc is the ground truth every verifier in this library diffs
+// against, so agreeing with it is what makes a disassembly comparable. Before
+// this table, 52 of the 74 aliased encodings decoded to a name llvm-mc does not
+// use (`0F 84` → JZ where it says JE, `A4` → MOVS where it says MOVSB, `DB E2` →
+// FCLEX where it says FNCLEX).
+//
+// An alias stays fully ENCODABLE — `inst_r_r(.SAL, …)` emits the same bytes it
+// always did. Only the decode direction is narrowed, and only where the
+// canonical name covers the byte-identical encoding: that condition is what lets
+// MOV be the alias at the `A0`-`A3`/`B8` moffs forms while remaining the only
+// name for `88`/`89`. `tablegen/gen.odin` applies it when it collects decode
+// entries, so an aliased mnemonic never reaches the decode tables at all.
+//
+// Adding an instruction whose mnemonic aliases another one's encoding requires a
+// row here; `run_alias_table_test` in tests/ recomputes the ambiguity from the
+// built tables and fails by name if one is missing, so it cannot be forgotten.
+
+Mnemonic_Alias :: struct {
+	alias:     Mnemonic, // never produced by the decoder
+	canonical: Mnemonic, // produced instead, at the byte-identical encoding
+}
+
+@(rodata)
+MNEMONIC_ALIASES := [?]Mnemonic_Alias{
+	// -- Jcc, both the short 0x7x and near 0x0F 8x forms --------------------
+	{.JNAE, .JB},  {.JC, .JB},
+	{.JNB, .JAE},  {.JNC, .JAE},
+	{.JZ, .JE},
+	{.JNZ, .JNE},
+	{.JNA, .JBE},
+	{.JNBE, .JA},
+	{.JPE, .JP},
+	{.JPO, .JNP},
+	{.JNGE, .JL},
+	{.JNL, .JGE},
+	{.JNG, .JLE},
+	{.JNLE, .JG},
+
+	// -- CMOVcc (0x0F 4x) ---------------------------------------------------
+	{.CMOVNAE, .CMOVB},  {.CMOVC, .CMOVB},
+	{.CMOVNB, .CMOVAE},  {.CMOVNC, .CMOVAE},
+	{.CMOVZ, .CMOVE},
+	{.CMOVNZ, .CMOVNE},
+	{.CMOVNA, .CMOVBE},
+	{.CMOVNBE, .CMOVA},
+	{.CMOVPE, .CMOVP},
+	{.CMOVPO, .CMOVNP},
+	{.CMOVNGE, .CMOVL},
+	{.CMOVNL, .CMOVGE},
+	{.CMOVNG, .CMOVLE},
+	{.CMOVNLE, .CMOVG},
+
+	// -- SETcc (0x0F 9x) ----------------------------------------------------
+	{.SETNAE, .SETB},  {.SETC, .SETB},
+	{.SETNB, .SETAE},  {.SETNC, .SETAE},
+	{.SETZ, .SETE},
+	{.SETNZ, .SETNE},
+	{.SETNA, .SETBE},
+	{.SETNBE, .SETA},
+	{.SETPE, .SETP},
+	{.SETPO, .SETNP},
+	{.SETNGE, .SETL},
+	{.SETNL, .SETGE},
+	{.SETNG, .SETLE},
+	{.SETNLE, .SETG},
+
+	// -- Shift group: SAL and SHL are both /4, the same encoding ------------
+	{.SAL, .SHL},
+
+	// -- String ops: the bare name against the explicitly byte-sized one ----
+	{.CMPS, .CMPSB},
+	{.LODS, .LODSB},
+	{.MOVS, .MOVSB},
+	{.SCAS, .SCASB},
+	{.STOS, .STOSB},
+
+	// -- x87: the assembler's wait-prefixed spelling of a no-wait opcode ----
+	// (FSTENV is really `9B D9 /6`; the table gives it the bare `D9 /6`, which
+	//  is FNSTENV. Modelling the 9B prefix is a separate question — until then
+	//  the bare encoding decodes as the no-wait name, which is what it is.)
+	{.FCLEX, .FNCLEX},
+	{.FINIT, .FNINIT},
+	{.FSAVE, .FNSAVE},
+	{.FSTCW, .FNSTCW},
+	{.FSTENV, .FNSTENV},
+	{.FSTSW, .FNSTSW},
+
+	// -- Odds --------------------------------------------------------------
+	{.FWAIT, .WAIT},
+	{.XLAT, .XLATB},
+	// MOV aliases MOVABS only at the moffs (`A0`-`A3`) and imm64 (`B8+r`)
+	// forms; the coverage rule leaves every other MOV encoding untouched.
+	{.MOV, .MOVABS},
+}
+
+// The name a disassembler prints for `m`'s encoding — `m` itself unless it is a
+// declared alias. Useful to an assembler front-end that accepts either spelling
+// and wants to compare against decoder output.
+canonical_mnemonic :: proc "contextless" (m: Mnemonic) -> Mnemonic {
+	for entry in MNEMONIC_ALIASES {
+		if entry.alias == m { return entry.canonical }
+	}
+	return m
+}
+
+// Is `m` a name the decoder never produces?
+is_mnemonic_alias :: proc "contextless" (m: Mnemonic) -> bool {
+	for entry in MNEMONIC_ALIASES {
+		if entry.alias == m { return true }
+	}
+	return false
+}

core/rexcode/isa/x86/decoder.odin

@@ -565,6 +565,21 @@ decode_opcode :: proc(state: ^Decoder_State) -> (entry: ^Decode_Entry, vex_entry
 		}
 
 		if !has_modrm_byte {
+			// ADDRESS-size variants first: a form declaring one is picked by which
+			// address size is in effect (mode default vs 67h), an axis the operand-
+			// size scan below cannot see -- it reads REX.W and 66h, and neither
+			// affects address size. 0xE3 is the case: JRCXZ/JECXZ/JCXZ are one
+			// opcode telling RCX/ECX/CX apart, and nothing else in the bytes says
+			// which. Checked before the operand-size pass so a REX.W-prefixed E3
+			// (legal, and ignored by the CPU for address size) still reads as JRCXZ.
+			addr_want := effective_addr_size(state.mode, state.prefix_67)
+			for i in 0..<int(idx.count) {
+				e := &LEGACY_DECODE_ENTRIES[base + i]
+				if e.flags.addr_size != .DEFAULT && e.flags.addr_size == addr_want {
+					return e, nil, .NONE
+				}
+			}
+
 			// Select the size variant. Two families live here: flag-tagged forms
 			// with no operands (CBW/CWDE/CDQE, CWD/CDQ/CQO, string ops, PUSHF/POPF,
 			// IRET*) and accumulator+immediate forms whose size shows in the implied

core/rexcode/isa/x86/encoder.odin

@@ -334,8 +334,26 @@ encode :: proc(
 				pos += 1
 			}
 
-			// Address size override (67h)
-			if inst.flags.addr32 {
+			// Address size override (67h), when the CALLER asked for one. A form
+			// that requires a particular address size emits it below instead --
+			// this branch is inside the "any instruction flag is set" gate, and
+			// such a form needs the prefix whether or not the caller set a flag.
+			if inst.flags.addr32 && enc.flags.addr_size == .DEFAULT {
+				out[pos] = 0x67
+				pos += 1
+			}
+		}
+
+		// A form whose ADDRESS size is fixed (JRCXZ/JECXZ/JCXZ -- one opcode, the
+		// mnemonic saying which counter register) carries the prefix when its size
+		// is not the mode's default. Outside the flags gate above because it is a
+		// property of the ENCODING, not of the caller's request; `addr_size` is
+		// .DEFAULT for all but three forms, so the test is one compare against a
+		// field already loaded.
+		if enc.flags.addr_size != .DEFAULT {
+			// Reachability was settled by the matcher's gate, which refuses a form
+			// whose address size this mode cannot express.
+			if needs_67, _ := addr_size_prefix(enc.flags.addr_size, mode); needs_67 {
 				out[pos] = 0x67
 				pos += 1
 			}
@@ -892,6 +910,15 @@ encoding_matches_inline :: proc "contextless" (inst: ^Instruction, enc: ^Encodin
 		return false
 	}
 
+	// Address-size gate: a form fixed to an address size this mode cannot reach
+	// is not encodable here at all -- JCXZ (16-bit) in long mode, where 67h
+	// selects 32-bit; JRCXZ (64-bit) outside it. Every other form is .DEFAULT.
+	if enc.flags.addr_size != .DEFAULT {
+		if _, reachable := addr_size_prefix(enc.flags.addr_size, mode); !reachable {
+			return false
+		}
+	}
+
 	// PUSH/POP FS/GS: the segment operand is fixed by the opcode (0F A0/A1 -> FS,
 	// 0F A8/A9 -> GS), so a form only matches when the user's segment agrees --
 	// otherwise `push gs` would take the first {SREG} form (FS) and mis-encode.

core/rexcode/isa/x86/encoding_types.odin

@@ -241,6 +241,48 @@ VEX_L :: enum u8 {
 	L2,             // L = 2 (512-bit, EVEX only)
 }
 
+/*
+	The ADDRESS size a form requires -- a different axis from operand size (66h /
+	REX.W), selected by 67h against the mode's default: 64-bit in long mode and
+	32-bit under 67h; 32-bit in protected mode and 16-bit under 67h.
+
+	Almost every instruction is DEFAULT (it works at whatever address size is in
+	effect). It matters where address size picks the MNEMONIC: 0xE3 is JRCXZ,
+	JECXZ or JCXZ purely by which counter register it tests, and nothing else in
+	the encoding says which. Modelling that as REX.W (which does not affect
+	address size at all) made `48 E3 cb` the encoding of JRCXZ, where the correct
+	one is a bare `E3 cb`, and left the decoder unable to tell the three apart.
+*/
+Addr_Size :: enum u8 {
+	DEFAULT,        // whatever the mode/prefix selects; the form does not care
+	A16,            // 16-bit addressing (CX)
+	A32,            // 32-bit addressing (ECX)
+	A64,            // 64-bit addressing (RCX); long mode only
+}
+
+// The address size in effect for `mode` with/without a 67h override.
+effective_addr_size :: #force_inline proc "contextless" (mode: Mode, prefix_67: bool) -> Addr_Size {
+	if mode == ._64 {
+		return prefix_67 ? .A32 : .A64
+	}
+	return prefix_67 ? .A16 : .A32
+}
+
+// Does reaching `want` in `mode` require a 67h prefix, and is it reachable at all?
+addr_size_prefix :: #force_inline proc "contextless" (want: Addr_Size, mode: Mode) -> (prefix_67: bool, ok: bool) {
+	if want == .DEFAULT {
+		return false, true
+	}
+	if want == effective_addr_size(mode, false) {
+		return false, true
+	}
+	if want == effective_addr_size(mode, true) {
+		return true, true
+	}
+	// e.g. JCXZ (A16) in long mode: 67h there gives 32-bit, never 16-bit.
+	return false, false
+}
+
 // -----------------------------------------------------------------------------
 // SECTION: 6.5 Encoding Flags
 // -----------------------------------------------------------------------------
@@ -259,6 +301,7 @@ Encoding_Flags :: bit_field u32 {
 	rep_ok:         bool     | 1, // REP prefix valid
 	modrm_reg_ext:  bool     | 1, // ModR/M reg field is opcode extension (use ext field)
 	mode_32_only:   bool     | 1, // only valid in Mode._32 (e.g. short-form INC/DEC at 0x40-0x4F)
+	addr_size:  Addr_Size    | 2, // required ADDRESS size (67h axis); selects JCXZ/JECXZ/JRCXZ
 
 	explicit_count: u8       | 3, // 0..<4 non-implicit operands
 	has_implicit:   bool     | 1, // any implicit operand
@@ -302,8 +345,9 @@ encoding_flags :: #force_inline proc "contextless" (
 	no_rex:        bool     = false,
 	lock_ok:       bool     = false,
 	rep_ok:        bool     = false,
-	modrm_reg_ext: bool     = false,
-	mode_32_only:  bool     = false,
+	modrm_reg_ext: bool      = false,
+	mode_32_only:  bool      = false,
+	addr_size:     Addr_Size = .DEFAULT,
 ) -> Encoding_Flags {
 	return Encoding_Flags{
 		esc           = esc,
@@ -319,6 +363,7 @@ encoding_flags :: #force_inline proc "contextless" (
 		rep_ok        = rep_ok,
 		modrm_reg_ext = modrm_reg_ext,
 		mode_32_only  = mode_32_only,
+		addr_size     = addr_size,
 	}
 }
 

core/rexcode/isa/x86/tablegen/gen.odin

@@ -267,6 +267,9 @@ emit_decode_tables :: proc() -> (n_legacy, n_vex, n_evex: int) {
 			}
 		}
 	}
+	drop_aliased(&legacy)
+	drop_aliased(&vex)
+	drop_aliased(&evex)
 	slice.sort_by(legacy[:], entry_less)
 	slice.sort_by(vex[:],    entry_less)
 	slice.sort_by(evex[:],   entry_less)
@@ -290,6 +293,73 @@ emit_decode_tables :: proc() -> (n_legacy, n_vex, n_evex: int) {
 	return len(legacy), len(vex), len(evex)
 }
 
+/*
+	Several mnemonics can name ONE encoding (SHL and SAL are both /4 in the shift
+	group; JE and JZ are both 0x74). The decoder reads bytes, so it cannot tell
+	them apart and must simply print one name -- which means the choice has to be
+	declared, or it falls out of wherever the entry lands in the sort below.
+
+	lib.MNEMONIC_ALIASES declares it. An alias entry is dropped here, before the
+	tables are written, so the name never reaches the decoder at all; it stays
+	fully encodable, since ENCODE_FORMS is built from the same table separately.
+
+	The drop is CONDITIONAL on the canonical name covering the byte-identical
+	encoding. That is what keeps the rule safe for a mnemonic that is an alias at
+	one opcode and the only name at another: MOV aliases MOVABS at the `A0`-`A3`
+	moffs and `B8+r` imm64 forms, and dropping it unconditionally would leave
+	`88`/`89` -- ordinary register MOV -- with no decode entry at all.
+*/
+drop_aliased :: proc(entries: ^[dynamic]Collected_Entry) {
+	Key :: struct {
+		esc:    lib.Escape,
+		prefix: u8,
+		opcode: u8,
+		ext:    u8,
+		ops:    [4]lib.Operand_Type,
+		enc:    [4]lib.Operand_Encoding,
+		flags:  lib.Encoding_Flags,
+		vex_w:  lib.VEX_W,
+		vex_l:  lib.VEX_L,
+	}
+	key_of :: proc(e: Collected_Entry) -> Key {
+		return Key{e.esc, e.prefix, e.opcode, e.ext, e.ops, e.enc, e.flags, e.vex_w, e.vex_l}
+	}
+	/* Which mnemonics each indistinguishable encoding is spelled by. */
+	names := make(map[Key][dynamic]lib.Mnemonic)
+	defer {
+		for _, list in names { delete(list) }
+		delete(names)
+	}
+	for e in entries {
+		key := key_of(e)
+		list, present := &names[key]
+		if !present {
+			names[key] = make([dynamic]lib.Mnemonic)
+			list = &names[key]
+		}
+		append(list, e.mnemonic)
+	}
+
+	kept := 0
+	for e in entries {
+		drop := false
+		for entry in lib.MNEMONIC_ALIASES {
+			if entry.alias != e.mnemonic { continue }
+			for spelling in names[key_of(e)] {
+				if spelling == entry.canonical {
+					drop = true
+					break
+				}
+			}
+			if drop { break }
+		}
+		if drop { continue }
+		entries[kept] = e
+		kept += 1
+	}
+	resize(entries, kept)
+}
+
 entry_less :: proc(a, b: Collected_Entry) -> bool {
 	if a.esc != b.esc       { return int(a.esc) < int(b.esc) }
 	if a.prefix != b.prefix { return a.prefix < b.prefix }
@@ -470,6 +540,12 @@ write_flags :: proc(sb: ^strings.Builder, enc: union{lib.Encoding, Collected_Ent
 	if flags.rep_ok                { append(&parts, "rep_ok=true") }
 	if flags.modrm_reg_ext         { append(&parts, "modrm_reg_ext=true") }
 	if flags.mode_32_only          { append(&parts, "mode_32_only=true") }
+	if flags.addr_size != .DEFAULT { append(&parts, fmt.tprintf("addr_size=.%v", flags.addr_size)) }
+	/* EVERY Encoding_Flags field must be listed above. This enumerates the
+	   struct by hand, so a field added there and forgotten here is silently
+	   dropped from the generated tables -- the flag reads back as its zero value
+	   and the instruction is quietly mis-modelled, with nothing to see in the
+	   diff but its absence. `addr_size` was added and did exactly that. */
 
 	switch e in enc {
 	case lib.Encoding:

core/rexcode/isa/x86/tablegen/generated/encode_tables.odin

@@ -585,11 +585,11 @@ ENCODE_FORMS := [2478]lib.Encoding{
 	{.JS,               {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0x78, 0, {explicit_count=1}},
 	{.JS,               {.REL32,     .NONE,      .NONE,      .NONE}, {.ID,   .NONE, .NONE, .NONE}, 0x88, 0, {esc=._0F, explicit_count=1}},
 	// .JCXZ
-	{.JCXZ,             {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE3, 0, {explicit_count=1}},
+	{.JCXZ,             {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE3, 0, {addr_size=.A16, explicit_count=1}},
 	// .JECXZ
-	{.JECXZ,            {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE3, 0, {explicit_count=1}},
+	{.JECXZ,            {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE3, 0, {addr_size=.A32, explicit_count=1}},
 	// .JRCXZ
-	{.JRCXZ,            {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE3, 0, {force_rex_w=true, explicit_count=1}},
+	{.JRCXZ,            {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE3, 0, {addr_size=.A64, explicit_count=1}},
 	// .LOOP
 	{.LOOP,             {.REL8,      .NONE,      .NONE,      .NONE}, {.IB,   .NONE, .NONE, .NONE}, 0xE2, 0, {explicit_count=1}},
 	// .LOOPE

core/rexcode/isa/x86/tablegen/instruction_table.odin

@@ -733,14 +733,20 @@ INSTRUCTION_TABLE := [Mnemonic][]Form{
 		{{.JS, {.REL8,  .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0x78, 0, {}},         {flags_rd={.SF}, side_effects={.CONTROL}}},
 		{{.JS, {.REL32, .NONE, .NONE, .NONE}, {.ID, .NONE, .NONE, .NONE}, 0x88, 0, {esc=._0F}}, {flags_rd={.SF}, side_effects={.CONTROL}}},
 	},
+	/* 0xE3 is ONE opcode whose mnemonic is chosen by ADDRESS size -- which
+	   counter register it tests -- not by operand size. So the three are told
+	   apart by `addr_size` (67h against the mode default), never by REX.W, which
+	   does not affect address size at all: JRCXZ is a bare `E3 cb` in long mode
+	   and JECXZ is `67 E3 cb`; in protected mode JECXZ is bare and JCXZ takes
+	   the prefix, while JCXZ is unreachable in long mode and JRCXZ outside it. */
 	.JCXZ = {
-		{{.JCXZ, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE3, 0, {}}, {implicit_rd={.RCX}, side_effects={.CONTROL}}},
+		{{.JCXZ, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE3, 0, {addr_size=.A16}}, {implicit_rd={.RCX}, side_effects={.CONTROL}}},
 	},
 	.JECXZ = {
-		{{.JECXZ, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE3, 0, {}}, {implicit_rd={.RCX}, side_effects={.CONTROL}}},
+		{{.JECXZ, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE3, 0, {addr_size=.A32}}, {implicit_rd={.RCX}, side_effects={.CONTROL}}},
 	},
 	.JRCXZ = {
-		{{.JRCXZ, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE3, 0, {force_rex_w=true}}, {implicit_rd={.RCX}, side_effects={.CONTROL}}},
+		{{.JRCXZ, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE3, 0, {addr_size=.A64}}, {implicit_rd={.RCX}, side_effects={.CONTROL}}},
 	},
 	.LOOP = {
 		{{.LOOP, {.REL8, .NONE, .NONE, .NONE}, {.IB, .NONE, .NONE, .NONE}, 0xE2, 0, {}}, {implicit_wr={.RCX}, implicit_rd={.RCX}, side_effects={.CONTROL}}},

core/rexcode/isa/x86/tests/test.odin

@@ -666,35 +666,195 @@ print_highlighted :: proc(text: string, tokens: []x86.Token) {
 // SECTION 4: MNEMONIC EQUIVALENCE
 // =============================================================================
 
+/*
+	Several x86 mnemonics name ONE encoding -- SHL and SAL are both /4 in the
+	shift group, JE and JZ are both 0x74. Which name a decode returns is declared
+	by x86.MNEMONIC_ALIASES and enforced by the generator, which drops the
+	aliased spelling before the decode tables are written; so encoding SAL and
+	decoding it back yields SHL, and a round-trip compares canonical forms.
+
+	This used to be a second alias table maintained BY HAND here, and that is how
+	SHL/SAL went missing: the shift group was never listed, nothing checked, and
+	it stayed invisible until a table regeneration moved SAL ahead of SHL and
+	four long-passing tests began reporting `SAL != expected SHL`. There is now
+	one table, in the library, and run_alias_table_test below proves it complete
+	against the built tables rather than against anyone's memory.
+*/
 mnemonics_eq :: proc(a, b: x86.Mnemonic) -> bool {
-	if a == b { return true }
-	aliases := [][2]x86.Mnemonic{
-		// MOV/MOVABS
-		{.MOV, .MOVABS},
-		// CMOVcc aliases
-		{.CMOVE, .CMOVZ}, {.CMOVNE, .CMOVNZ},
-		{.CMOVG, .CMOVNLE}, {.CMOVGE, .CMOVNL},
-		{.CMOVL, .CMOVNGE}, {.CMOVLE, .CMOVNG},
-		{.CMOVA, .CMOVNBE}, {.CMOVAE, .CMOVNB}, {.CMOVAE, .CMOVNC},
-		{.CMOVB, .CMOVNAE}, {.CMOVB, .CMOVC}, {.CMOVBE, .CMOVNA},
-		// SETcc aliases
-		{.SETE, .SETZ}, {.SETNE, .SETNZ},
-		{.SETG, .SETNLE}, {.SETGE, .SETNL},
-		{.SETL, .SETNGE}, {.SETLE, .SETNG},
-		{.SETA, .SETNBE}, {.SETAE, .SETNB}, {.SETAE, .SETNC},
-		{.SETB, .SETNAE}, {.SETB, .SETC}, {.SETBE, .SETNA},
-		// Jcc aliases
-		{.JE, .JZ}, {.JNE, .JNZ},
-		{.JB, .JC}, {.JAE, .JNC},
-		{.JG, .JNLE}, {.JGE, .JNL},
-		{.JL, .JNGE}, {.JLE, .JNG},
-		{.JA, .JNBE}, {.JAE, .JNB},
-		{.JB, .JNAE}, {.JBE, .JNA},
+	return a == b || x86.canonical_mnemonic(a) == x86.canonical_mnemonic(b)
+}
+
+/*
+	No encoding may be spelled by two mnemonics that both survive into the decode
+	tables -- because the decoder would then have to choose between them with
+	nothing to choose on, which is the defect x86.MNEMONIC_ALIASES exists to
+	remove. Two entries are indistinguishable exactly when they agree on every
+	field the decoder can read, so the property is computable from the tables
+	themselves, and this recomputes it.
+
+	It is what makes the declaration maintainable: an instruction added with a
+	mnemonic that aliases another's encoding fails here BY NAME, with the row to
+	write, instead of silently decoding as whichever spelling the sort happened
+	to put first.
+*/
+run_alias_table_test :: proc() {
+	Group_Key :: struct {
+		esc:    x86.Escape,
+		prefix: u8,
+		opcode: u8,
+		ext:    u8,
+		ops:    [4]x86.Operand_Type,
+		enc:    [4]x86.Operand_Encoding,
+		flags:  x86.Encoding_Flags,
 	}
-	for alias in aliases {
-		if (a == alias[0] && b == alias[1]) || (a == alias[1] && b == alias[0]) { return true }
+	groups := make(map[Group_Key][dynamic]x86.Mnemonic)
+	defer {
+		for _, list in groups { delete(list) }
+		delete(groups)
+	}
+	for e in x86.LEGACY_DECODE_ENTRIES {
+		key := Group_Key{e.esc, e.prefix, e.opcode, e.ext, e.ops, e.enc, e.flags}
+		list, present := &groups[key]
+		if !present {
+			groups[key] = make([dynamic]x86.Mnemonic)
+			list = &groups[key]
+		}
+		known := false
+		for m in list {
+			if m == e.mnemonic {
+				known = true
+				break
+			}
+		}
+		if !known { append(list, e.mnemonic) }
+	}
+
+	ambiguous := 0
+	for key, list in groups {
+		if len(list) < 2 { continue }
+		ambiguous += 1
+		if ambiguous <= 10 {
+			fmt.printf("    %sFAIL%s esc=%v prefix=%d opcode=%02X ext=%02X is spelled by",
+				RED, RESET, key.esc, key.prefix, key.opcode, key.ext)
+			for m in list { fmt.printf(" %v", m) }
+			fmt.printf(",\n         and the decoder has nothing to choose between them. Declare one in\n")
+			fmt.printf("         x86.MNEMONIC_ALIASES (isa/x86/aliases.odin), then regenerate:\n")
+			fmt.printf("           odin run core/rexcode/isa/x86/tablegen && odin run core/rexcode/isa/x86/tablegen/generated\n")
+		}
+	}
+	if ambiguous > 10 {
+		fmt.printf("         ... and %d further ambiguous encoding(s).\n", ambiguous - 10)
+	}
+	if ambiguous > 0 {
+		g_stats.failed += 1
+		return
+	}
+	g_stats.passed += 1
+	g_stats.cases_validated += 1
+}
+
+/*
+	0xE3 is one opcode whose MNEMONIC is chosen by address size -- which counter
+	register it tests -- and address size is the 67h axis, not REX.W. Modelling
+	JRCXZ as `force_rex_w` made its encoding `48 E3 cb` (llvm-mc emits a bare
+	`E3 cb`, and REX.W does not affect address size at all), gave JECXZ the bare
+	encoding that is really JRCXZ in long mode, and left the decoder with three
+	indistinguishable entries to guess between.
+
+	Both directions and both modes are asserted here, refusals included: JCXZ is
+	unreachable in long mode (67h selects 32-bit there, never 16-bit) and JRCXZ
+	outside it. Every expectation below was measured against llvm-mc.
+*/
+run_addr_size_tests :: proc() {
+	Decode_Case :: struct {
+		bytes: []u8,
+		mode:  x86.Mode,
+		want:  x86.Mnemonic,
+		note:  string,
+	}
+	decodes := []Decode_Case{
+		{{0xE3, 0x00},       ._64, .JRCXZ, "long mode default address size is 64-bit"},
+		{{0x67, 0xE3, 0x00}, ._64, .JECXZ, "67h drops long mode to 32-bit"},
+		{{0x48, 0xE3, 0x00}, ._64, .JRCXZ, "REX.W is ignored for address size"},
+		{{0xE3, 0x00},       ._32, .JECXZ, "protected mode default is 32-bit"},
+		{{0x67, 0xE3, 0x00}, ._32, .JCXZ,  "67h drops protected mode to 16-bit"},
+	}
+	for c in decodes {
+		insts  := make([dynamic]x86.Instruction)
+		info   := make([dynamic]x86.Instruction_Info)
+		labels := make([dynamic]x86.Label_Definition)
+		errors := make([dynamic]x86.Error)
+		defer {
+			delete(insts); delete(info); delete(labels); delete(errors)
+		}
+		_, ok := x86.decode(c.bytes, nil, &insts, &info, &labels, &errors, c.mode)
+		if !ok || len(insts) == 0 {
+			g_stats.failed += 1
+			fmt.printf("    %sFAIL%s %v decode failed in %v (%s)\n", RED, RESET, c.bytes, c.mode, c.note)
+			continue
+		}
+		if insts[0].mnemonic != c.want {
+			g_stats.failed += 1
+			fmt.printf("    %sFAIL%s %v in %v decoded %v, expected %v -- %s\n",
+				RED, RESET, c.bytes, c.mode, insts[0].mnemonic, c.want, c.note)
+			continue
+		}
+		g_stats.passed += 1
+		g_stats.cases_validated += 1
+	}
+
+	Encode_Case :: struct {
+		mnemonic: x86.Mnemonic,
+		mode:     x86.Mode,
+		want:     []u8, // empty = must be refused; this mode cannot express it
+	}
+	encodes := []Encode_Case{
+		{.JRCXZ, ._64, {0xE3, 0x00}},
+		{.JECXZ, ._64, {0x67, 0xE3, 0x00}},
+		{.JCXZ,  ._64, {}},
+		{.JECXZ, ._32, {0xE3, 0x00}},
+		{.JCXZ,  ._32, {0x67, 0xE3, 0x00}},
+		{.JRCXZ, ._32, {}},
+	}
+	for c in encodes {
+		insts  := []x86.Instruction{x86.inst_rel_offset(c.mnemonic, 0, 1)}
+		code   := make([]u8, 32)
+		relocs := make([dynamic]x86.Relocation)
+		errors := make([dynamic]x86.Error)
+		defer {
+			delete(code); delete(relocs); delete(errors)
+		}
+		count, ok := x86.encode(insts, nil, code, &relocs, &errors, true, 0, c.mode)
+		if len(c.want) == 0 {
+			if ok && count > 0 {
+				g_stats.failed += 1
+				fmt.printf("    %sFAIL%s %v encoded to %02X in %v, but that mode cannot express its address size\n",
+					RED, RESET, c.mnemonic, code[:count], c.mode)
+				continue
+			}
+			g_stats.passed += 1
+			g_stats.cases_validated += 1
+			continue
+		}
+		if !ok || int(count) != len(c.want) {
+			g_stats.failed += 1
+			fmt.printf("    %sFAIL%s %v in %v encoded %d byte(s), expected %d\n",
+				RED, RESET, c.mnemonic, c.mode, count, len(c.want))
+			continue
+		}
+		same := true
+		for want, i in c.want {
+			if code[i] != want { same = false; break }
+		}
+		if !same {
+			g_stats.failed += 1
+			fmt.printf("    %sFAIL%s %v in %v encoded % 02X, expected % 02X\n",
+				RED, RESET, c.mnemonic, c.mode, code[:count], c.want)
+			continue
+		}
+		g_stats.passed += 1
+		g_stats.cases_validated += 1
 	}
-	return false
 }
 
 // =============================================================================
@@ -3630,6 +3790,12 @@ main :: proc() {
 	log_header("TYPED BUILDER CONSISTENCY")
 	run_typed_builder_tests()
 
+	log_header("MNEMONIC ALIAS TABLE")
+	run_alias_table_test()
+
+	log_header("ADDRESS-SIZE SELECTED MNEMONICS")
+	run_addr_size_tests()
+
 	log_header("I386 (32-BIT MODE) TESTS")
 	run_i386_tests()