From b9983105bdcaa1d4739792aec8e3596799e1ab66 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Wed, 19 Aug 2026 14:03:56 -0700 Subject: [PATCH 1/3] amd64 asm: extend family emits mnemonic that doesnt exist --- src/llvm_backend_asm.cpp | 58 ++++++++++++++++++++++++++++- tests/internal/test_asm_extend.odin | 40 ++++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/internal/test_asm_extend.odin diff --git a/src/llvm_backend_asm.cpp b/src/llvm_backend_asm.cpp index 94a825709..7c4e44103 100644 --- a/src/llvm_backend_asm.cpp +++ b/src/llvm_backend_asm.cpp @@ -185,6 +185,10 @@ struct lbAsmGenerate { } virtual char instruction_size_suffix(AstAsmInstruction *instr) = 0; + // Some AT&T mnemonics encode BOTH operand widths and so cannot be spelled as a name plus one + // suffix: `movsx` from i8 to i32 is `movsbl`. Returns the complete mnemonic, or {} when the + // name-plus-suffix spelling is the right one + virtual String instruction_att_mnemonic(AstAsmInstruction *instr) = 0; virtual char size_suffix_for_operand(Ast *op) = 0; virtual gbString write_memory_operand(gbString asm_string, Slice const &op_number, AstAsmMemoryOperand *mem_op, u32 flags) = 0; virtual lbValue emit_call(lbProcedure *p, Array const &args) = 0; @@ -221,6 +225,50 @@ struct lbAsmGenerate_amd64 : lbAsmGenerate { } + // The sign/zero-extend family is the only one whose two operands differ in width. Its AT&T + // mnemonic names both: `movsx` i8 -> i32 is `movsbl`, never `movsx` plus a suffix. `movsxd` is + // the same rule, movs + l + q + String instruction_att_mnemonic(AstAsmInstruction *instr) override { + bool sign_extend; + switch (instr->mnemonic) { + case Asm_amd64::M_MOVSX: + case Asm_amd64::M_MOVSXD: + sign_extend = true; + break; + case Asm_amd64::M_MOVZX: + sign_extend = false; + break; + default: + return {}; + } + + auto forms = g_asm_amd64.encoding_forms(instr->mnemonic); + if (instr->valid_form_index < 0 || instr->valid_form_index >= forms.count) { + return {}; + } + auto const &form = forms[instr->valid_form_index]; + + // Intel operand order: dst first + i32 dst = g_asm_amd64.operand_type_bit_width(form.ops[0]); + i32 src = g_asm_amd64.operand_type_bit_width(form.ops[1]); + + if (sign_extend) { + if (src == 8 && dst == 16) { return str_lit("movsbw"); } + if (src == 8 && dst == 32) { return str_lit("movsbl"); } + if (src == 8 && dst == 64) { return str_lit("movsbq"); } + if (src == 16 && dst == 32) { return str_lit("movswl"); } + if (src == 16 && dst == 64) { return str_lit("movswq"); } + if (src == 32 && dst == 64) { return str_lit("movslq"); } + } else { + if (src == 8 && dst == 16) { return str_lit("movzbw"); } + if (src == 8 && dst == 32) { return str_lit("movzbl"); } + if (src == 8 && dst == 64) { return str_lit("movzbq"); } + if (src == 16 && dst == 32) { return str_lit("movzwl"); } + if (src == 16 && dst == 64) { return str_lit("movzwq"); } + } + return {}; + } + // Scan an instruction's operands for an annotated memory operand and return its // size suffix, or 0 if none. The checker has already verified the annotation // agrees with the matched encoding form, so a suffix here can never conflict. @@ -508,6 +556,10 @@ struct lbAsmGenerate_amd64 : lbAsmGenerate { case_ast_node(instr, AsmInstruction, instr_); asm_string = gb_string_appendc(asm_string, "\t"); String name = instr->name->Ident.token.string; + String att = this->instruction_att_mnemonic(instr); + if (att.len != 0) { + name = att; + } asm_string = gb_string_append_length(asm_string, name.text, name.len); // If a memory operand carries an explicit size annotation ([p]:u8) and @@ -515,8 +567,10 @@ struct lbAsmGenerate_amd64 : lbAsmGenerate { // encoded as a mnemonic suffix (crc32 -> crc32b). The checker has already // verified the annotation agrees with the matched form, so an emitted // suffix can never conflict with a register operand's implied width. - if (char suffix = this->instruction_size_suffix(instr)) { - asm_string = gb_string_append_length(asm_string, &suffix, 1); + if (att.len == 0) { + if (char suffix = this->instruction_size_suffix(instr)) { + asm_string = gb_string_append_length(asm_string, &suffix, 1); + } } asm_string = gb_string_appendc(asm_string, " "); diff --git a/tests/internal/test_asm_extend.odin b/tests/internal/test_asm_extend.odin new file mode 100644 index 000000000..8a93efdd5 --- /dev/null +++ b/tests/internal/test_asm_extend.odin @@ -0,0 +1,40 @@ +package test_internal + +import "core:testing" + +// The sign/zero-extend family is the only one whose two operands differ in width, so its AT&T +// mnemonic names both -- `movsx` from i8 to i32 is `movsbl`. Emitting the name plus a single width +// suffix produced `movsxl`, which no assembler has, so none of these forms could be built +@(test) +asm_extend_mnemonics :: proc(t: ^testing.T) { + when ODIN_ARCH == .amd64 { + sx_8_16 :: asm(a: i8) -> (r: i16) { movsx r, a; } + sx_8_32 :: asm(a: i8) -> (r: i32) { movsx r, a; } + sx_8_64 :: asm(a: i8) -> (r: i64) { movsx r, a; } + sx_16_32 :: asm(a: i16) -> (r: i32) { movsx r, a; } + sx_16_64 :: asm(a: i16) -> (r: i64) { movsx r, a; } + zx_8_16 :: asm(a: u8) -> (r: u16) { movzx r, a; } + zx_8_32 :: asm(a: u8) -> (r: u32) { movzx r, a; } + zx_8_64 :: asm(a: u8) -> (r: u64) { movzx r, a; } + zx_16_32 :: asm(a: u16) -> (r: u32) { movzx r, a; } + zx_16_64 :: asm(a: u16) -> (r: u64) { movzx r, a; } + sxd :: asm(a: i32) -> (r: i64) { movsxd r, a; } + + testing.expect_value(t, sx_8_16(-1), i16(-1)) + testing.expect_value(t, sx_8_32(-1), i32(-1)) + testing.expect_value(t, sx_8_64(-1), i64(-1)) + testing.expect_value(t, sx_16_32(-300), i32(-300)) + testing.expect_value(t, sx_16_64(-300), i64(-300)) + testing.expect_value(t, sxd(-123456), i64(-123456)) + + testing.expect_value(t, zx_8_16(0xFF), u16(255)) + testing.expect_value(t, zx_8_32(0xFF), u32(255)) + testing.expect_value(t, zx_8_64(0xFF), u64(255)) + testing.expect_value(t, zx_16_32(0xFFFF), u32(65535)) + testing.expect_value(t, zx_16_64(0xFFFF), u64(65535)) + + // positive values too, so a mnemonic that merely truncates cannot pass + testing.expect_value(t, sx_8_32(127), i32(127)) + testing.expect_value(t, zx_8_32(1), u32(1)) + } +} From 26ae28dfb784dfccfa25546eb6326be570ae0922 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Wed, 19 Aug 2026 14:32:03 -0700 Subject: [PATCH 2/3] asm: multi-instruction template computed with one operand twice --- src/llvm_backend_asm.cpp | 4 ++- tests/internal/test_asm_operands.odin | 35 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/internal/test_asm_operands.odin diff --git a/src/llvm_backend_asm.cpp b/src/llvm_backend_asm.cpp index 94a825709..32e5076e5 100644 --- a/src/llvm_backend_asm.cpp +++ b/src/llvm_backend_asm.cpp @@ -437,7 +437,9 @@ struct lbAsmGenerate_amd64 : lbAsmGenerate { // Register output: '=' ['&'] ( '{pin}' | class-letter ) raw("="); - if (is_alloc_scratch) { // early-clobber: keep scratch off any input reg + // early-clobber: keep scratch, and any output a later instruction could read past, + // off an input's register. One instruction reads before it writes, so it is safe + if (is_alloc_scratch || tmpl_node->instructions.count > 1) { raw("&"); } if (e.pin.len != 0) { diff --git a/tests/internal/test_asm_operands.odin b/tests/internal/test_asm_operands.odin new file mode 100644 index 000000000..4bf05a54c --- /dev/null +++ b/tests/internal/test_asm_operands.odin @@ -0,0 +1,35 @@ +#+build amd64 +package test_internal + +import "core:fmt" +import "core:testing" + +// The `asm` restriction to amd64 is enforced in the parser, so a `when ODIN_ARCH` guard inside the +// body cannot suppress it -- the body is parsed either way. The file needs a build tag instead. +// +// A template of more than one instruction can write its output before a later instruction has read +// its inputs, so the output must not share a register with one of them. Without an early-clobber the +// allocator was free to overlap them and `mov r, a; add r, b` computed `a + a`. +// +// The shape below matters: the defect only appears when surrounding register pressure makes the +// allocator pick an input's register for the output, and two asm calls as arguments to one variadic +// call is what did it. The same calls in separate statements were always correct +@(test) +asm_multi_instruction_output_does_not_alias_an_input :: proc(t: ^testing.T) { + add32 :: asm(a: i32, b: i32) -> (r: i32) { mov r, a; add r, b; } + sub32 :: asm(a: i32, b: i32) -> (r: i32) { mov r, a; sub r, b; } + mul32 :: asm(a: i32, b: i32) -> (r: i32) { mov r, a; imul r, b; } + xor64 :: asm(a: u64, b: u64) -> (r: u64) { mov r, a; xor r, b; } + wide :: asm(a: i64) -> (r: i64) { mov r, a; } + + testing.expect_value(t, fmt.tprintf("%v %v", add32(20, 22), wide(-7)), "42 -7") + testing.expect_value(t, fmt.tprintf("%v %v", sub32(50, 8), wide(-7)), "42 -7") + testing.expect_value(t, fmt.tprintf("%v %v", mul32(6, 7), wide(-7)), "42 -7") + testing.expect_value(t, fmt.tprintf("%v %v", xor64(0xF0, 0x0F), wide(-7)), "255 -7") + + // asymmetric arguments, so `a + a` or `a - a` cannot pass by coincidence + testing.expect_value(t, add32(1, 100), i32(101)) + testing.expect_value(t, sub32(1, 100), i32(-99)) + testing.expect_value(t, mul32(1, 100), i32(100)) + testing.expect_value(t, wide(-7), i64(-7)) +} From 40a64a9dbb654def75a64a7322c3c156b8b34481 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Wed, 19 Aug 2026 14:39:12 -0700 Subject: [PATCH 3/3] move test to build specific --- tests/internal/test_asm_extend.odin | 53 ++++++++++++++--------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/tests/internal/test_asm_extend.odin b/tests/internal/test_asm_extend.odin index 8a93efdd5..489ae67cb 100644 --- a/tests/internal/test_asm_extend.odin +++ b/tests/internal/test_asm_extend.odin @@ -1,3 +1,4 @@ +#+build amd64 package test_internal import "core:testing" @@ -7,34 +8,32 @@ import "core:testing" // suffix produced `movsxl`, which no assembler has, so none of these forms could be built @(test) asm_extend_mnemonics :: proc(t: ^testing.T) { - when ODIN_ARCH == .amd64 { - sx_8_16 :: asm(a: i8) -> (r: i16) { movsx r, a; } - sx_8_32 :: asm(a: i8) -> (r: i32) { movsx r, a; } - sx_8_64 :: asm(a: i8) -> (r: i64) { movsx r, a; } - sx_16_32 :: asm(a: i16) -> (r: i32) { movsx r, a; } - sx_16_64 :: asm(a: i16) -> (r: i64) { movsx r, a; } - zx_8_16 :: asm(a: u8) -> (r: u16) { movzx r, a; } - zx_8_32 :: asm(a: u8) -> (r: u32) { movzx r, a; } - zx_8_64 :: asm(a: u8) -> (r: u64) { movzx r, a; } - zx_16_32 :: asm(a: u16) -> (r: u32) { movzx r, a; } - zx_16_64 :: asm(a: u16) -> (r: u64) { movzx r, a; } - sxd :: asm(a: i32) -> (r: i64) { movsxd r, a; } + sx_8_16 :: asm(a: i8) -> (r: i16) { movsx r, a; } + sx_8_32 :: asm(a: i8) -> (r: i32) { movsx r, a; } + sx_8_64 :: asm(a: i8) -> (r: i64) { movsx r, a; } + sx_16_32 :: asm(a: i16) -> (r: i32) { movsx r, a; } + sx_16_64 :: asm(a: i16) -> (r: i64) { movsx r, a; } + zx_8_16 :: asm(a: u8) -> (r: u16) { movzx r, a; } + zx_8_32 :: asm(a: u8) -> (r: u32) { movzx r, a; } + zx_8_64 :: asm(a: u8) -> (r: u64) { movzx r, a; } + zx_16_32 :: asm(a: u16) -> (r: u32) { movzx r, a; } + zx_16_64 :: asm(a: u16) -> (r: u64) { movzx r, a; } + sxd :: asm(a: i32) -> (r: i64) { movsxd r, a; } - testing.expect_value(t, sx_8_16(-1), i16(-1)) - testing.expect_value(t, sx_8_32(-1), i32(-1)) - testing.expect_value(t, sx_8_64(-1), i64(-1)) - testing.expect_value(t, sx_16_32(-300), i32(-300)) - testing.expect_value(t, sx_16_64(-300), i64(-300)) - testing.expect_value(t, sxd(-123456), i64(-123456)) + testing.expect_value(t, sx_8_16(-1), i16(-1)) + testing.expect_value(t, sx_8_32(-1), i32(-1)) + testing.expect_value(t, sx_8_64(-1), i64(-1)) + testing.expect_value(t, sx_16_32(-300), i32(-300)) + testing.expect_value(t, sx_16_64(-300), i64(-300)) + testing.expect_value(t, sxd(-123456), i64(-123456)) - testing.expect_value(t, zx_8_16(0xFF), u16(255)) - testing.expect_value(t, zx_8_32(0xFF), u32(255)) - testing.expect_value(t, zx_8_64(0xFF), u64(255)) - testing.expect_value(t, zx_16_32(0xFFFF), u32(65535)) - testing.expect_value(t, zx_16_64(0xFFFF), u64(65535)) + testing.expect_value(t, zx_8_16(0xFF), u16(255)) + testing.expect_value(t, zx_8_32(0xFF), u32(255)) + testing.expect_value(t, zx_8_64(0xFF), u64(255)) + testing.expect_value(t, zx_16_32(0xFFFF), u32(65535)) + testing.expect_value(t, zx_16_64(0xFFFF), u64(65535)) - // positive values too, so a mnemonic that merely truncates cannot pass - testing.expect_value(t, sx_8_32(127), i32(127)) - testing.expect_value(t, zx_8_32(1), u32(1)) - } + // positive values too, so a mnemonic that merely truncates cannot pass + testing.expect_value(t, sx_8_32(127), i32(127)) + testing.expect_value(t, zx_8_32(1), u32(1)) }