From deb45f3e6a4fed083c3665454e012def3f98b130 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Sun, 23 Aug 2026 21:34:31 -0700 Subject: [PATCH] asm named slots --- .../riscv/tablegen/cpp-compiler/cpp-gen.odin | 16 ++++++++ .../x86/tablegen/cpp-compiler/cpp-gen.odin | 30 ++++++++++++++ src/asm_tables_amd64.cpp | 25 +++++++++++ src/asm_tables_riscv.cpp | 11 +++++ src/check_asm.cpp | 30 +++++++++++++- tests/issues/run.bat | 1 + tests/issues/run.sh | 10 +++++ .../test_issue_asm_named_register_slot.odin | 41 +++++++++++++++++++ 8 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 tests/issues/test_issue_asm_named_register_slot.odin diff --git a/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin b/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin index dfe6caca3..8ab2a4a00 100644 --- a/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/riscv/tablegen/cpp-compiler/cpp-gen.odin @@ -663,6 +663,22 @@ main :: proc() { """) + strings.write_string(&sb, "\n\n") + + strings.write_string(&sb, """ + // RISC-V has no slot that only one named hardware register can fill. + u16 operand_type_named_reg_class(OperandType t) const { + gb_unused(t); + return REG_CLASS_NONE; + } + + String named_reg_class_string(u16 reg_class) const { + gb_unused(reg_class); + return str_lit("hardware"); + } + """) + + strings.write_string(&sb, "\n\n") strings.write_string(&sb, """ diff --git a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin index 554b9b6bb..fa29a6145 100644 --- a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin @@ -682,6 +682,36 @@ main :: proc() { """) + strings.write_string(&sb, "\n\n") + + strings.write_string(&sb, """ + // Slots only a specific named hardware register can fill. They carry no GPR/vector + // class and, apart from OP_MM, no width either. Nothing else in the size/class + // check constrains them and a template parameter would otherwise slip through. + u16 operand_type_named_reg_class(OperandType t) const { + switch (t) { + case OP_SREG: return REG_CLASS_SEG; + case OP_CR: return REG_CLASS_CR; + case OP_DR: return REG_CLASS_DR; + case OP_STI: return REG_CLASS_ST; + case OP_MM: return REG_CLASS_MM; + } + return REG_CLASS_NONE; + } + + String named_reg_class_string(u16 reg_class) const { + switch (reg_class) { + case REG_CLASS_SEG: return str_lit("segment"); + case REG_CLASS_CR: return str_lit("control"); + case REG_CLASS_DR: return str_lit("debug"); + case REG_CLASS_ST: return str_lit("x87 stack"); + case REG_CLASS_MM: return str_lit("MMX"); + } + return str_lit("hardware"); + } + """) + + strings.write_string(&sb, "\n\n") strings.write_string(&sb, """ diff --git a/src/asm_tables_amd64.cpp b/src/asm_tables_amd64.cpp index dab07eec7..6bc2d5806 100644 --- a/src/asm_tables_amd64.cpp +++ b/src/asm_tables_amd64.cpp @@ -695,6 +695,31 @@ struct Asm_amd64 { return AsmRegClass_Unknown; // OP_M*, OP_IMM*, OP_REL*, OP_SREG/CR/DR/MM/STi, moffs, ptr, m16_16... : no GPR/XMM class constraint here } + // Slots only a specific named hardware register can fill. They carry no GPR/vector + // class and, apart from OP_MM, no width either. Nothing else in the size/class + // check constrains them and a template parameter would otherwise slip through. + u16 operand_type_named_reg_class(OperandType t) const { + switch (t) { + case OP_SREG: return REG_CLASS_SEG; + case OP_CR: return REG_CLASS_CR; + case OP_DR: return REG_CLASS_DR; + case OP_STI: return REG_CLASS_ST; + case OP_MM: return REG_CLASS_MM; + } + return REG_CLASS_NONE; + } + + String named_reg_class_string(u16 reg_class) const { + switch (reg_class) { + case REG_CLASS_SEG: return str_lit("segment"); + case REG_CLASS_CR: return str_lit("control"); + case REG_CLASS_DR: return str_lit("debug"); + case REG_CLASS_ST: return str_lit("x87 stack"); + case REG_CLASS_MM: return str_lit("MMX"); + } + return str_lit("hardware"); + } + u16 operand_type_bit_width(OperandType t) const { switch (t) { case OP_R8: case OP_RM8: case OP_M8: case OP_AL_IMPL: case OP_CL_IMPL: case OP_K_M8: return 8; diff --git a/src/asm_tables_riscv.cpp b/src/asm_tables_riscv.cpp index 02deec215..e22e70364 100644 --- a/src/asm_tables_riscv.cpp +++ b/src/asm_tables_riscv.cpp @@ -618,6 +618,17 @@ struct Asm_riscv { return reg_class_from_operand_type(t); } + // RISC-V has no slot that only one named hardware register can fill. + u16 operand_type_named_reg_class(OperandType t) const { + gb_unused(t); + return REG_CLASS_NONE; + } + + String named_reg_class_string(u16 reg_class) const { + gb_unused(reg_class); + return str_lit("hardware"); + } + u16 operand_type_bit_width(OperandType t) const { switch (t) { case OP_NONE: diff --git a/src/check_asm.cpp b/src/check_asm.cpp index 53e758092..19d35bf58 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -152,6 +152,7 @@ enum AsmMismatch : u8 { AsmMismatch_Class, // register class mismatch AsmMismatch_ImmRange, // constant immediate does not fit the slot width AsmMismatch_ImmType, // non-integer constant where an integer immediate is required + AsmMismatch_NamedReg, // slot only a named hardware register can fill }; // Does a constant immediate value fit a slot of `bits` width (0 == unconstrained)? @@ -241,6 +242,23 @@ gb_internal bool check_asm_operand_size_class(AsmCtx *asm_ctx, typename AsmCtx:: AsmRegClass want_class = asm_ctx->operand_type_reg_class(slot); i32 want_w = asm_ctx->operand_type_bit_width(slot); + // A slot only a named hardware register can fill (segment/control/debug/x87/MMX) + // carries no class and, apart from MMX, no width either. Nothing below would + // reject a template parameter standing in for one. + u16 want_named = asm_ctx->operand_type_named_reg_class(slot); + if (want_named != 0) { + bool ok = false; + if (operand->expr != nullptr && operand->expr->kind == Ast_AsmRegister) { + auto r = asm_ctx->register_lookup(operand->expr->AsmRegister.name.string); + ok = r && asm_ctx->reg_class(asm_ctx->register_codes[r]) == want_named; + } + if (!ok) { + if (mismatch_) *mismatch_ = AsmMismatch_NamedReg; + return false; + } + return true; + } + // A pure-label / sizeless slot imposes no reg width/class. if (want_class == AsmRegClass_Unknown && want_w == 0) { return true; @@ -1366,13 +1384,13 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm width_pref += cast(int)asm_ctx->operand_type_bit_width(type); bool spot_ok = false; + AsmMismatch m = AsmMismatch_None; if (kind_ok) { bool mem_unsized = (src == AsmOperand_Memory) && are_types_identical(operand->type, t_rawptr); if (dst == AsmOperand_Register_Or_Memory && src == AsmOperand_Memory && mem_unsized) { spot_ok = true; // memory form accepts memory; no size check } else { - AsmMismatch m = AsmMismatch_None; i32 wb_ = 0, gb_ = 0; spot_ok = check_asm_operand_size_class(asm_ctx, type, operand, &m, &wb_, &gb_); if (!spot_ok && (m == AsmMismatch_Size || m == AsmMismatch_ImmRange) && wb_ > 0 && gb_ > 0) { @@ -1385,7 +1403,9 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm if (spot_ok) { score += 2; valid_spots[i] = true; - } else if (kind_ok) { + } else if (kind_ok && m != AsmMismatch_NamedReg) { + // A slot wanting a named hardware register is not a near miss for anything + // else, so it must not outrank a form that merely has the widths wrong. score += 1; // kind matched, only value/size/class failed } } @@ -1654,6 +1674,12 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, Entity *tm LIT(name), i, want_bits[i], LIT(asm_reg_class_strings[dst_reg_class]), LIT(asm_operand_kind_strings[dst]), got_bits[i], LIT(asm_reg_class_strings[src_reg_class]), LIT(asm_operand_kind_strings[src])); + } else if (m == AsmMismatch_NamedReg) { + auto slot = operand_slot_type(forms[best_form], cast(int)i); + error(operands[i].expr, "'%.*s' operand-%td must be a named %.*s register, got a %.*s", + LIT(name), i, + LIT(asm_ctx->named_reg_class_string(asm_ctx->operand_type_named_reg_class(slot))), + LIT(asm_operand_kind_strings[src])); } else if (dst == AsmOperand_Immediate) { error(operands[i].expr, "'%.*s' operand-%td must be an assemble-time constant or a $ immediate parameter, got a %.*s", LIT(name), i, LIT(asm_operand_kind_strings[src])); diff --git a/tests/issues/run.bat b/tests/issues/run.bat index 4cbb424b4..471c66526 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -43,6 +43,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin test ..\test_issue_7008.odin %COMMON% || exit /b ..\..\..\odin check ..\test_issue_7012.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_7260.odin -no-entry-point %COMMON% || exit /b +..\..\..\odin check ..\test_issue_asm_named_register_slot.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "8" || exit /b ..\..\..\odin check ..\test_issue_ellipsis_type_call.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "10" || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration_mismatch.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 98c144d9c..cea89f310 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -95,6 +95,16 @@ $ODIN test ../test_issue_7356.odin $COMMON $ODIN build ../test_issue_7167.odin $COMMON $ODIN build ../test_issue_7188.odin $COMMON $ODIN check ../test_issue_7260.odin -no-entry-point $COMMON_CHECK + +# `asm` templates are amd64-only, so this file is empty on every other architecture +if [[ "$(uname -m)" == "x86_64" || "$(uname -m)" == "amd64" ]]; then + if [[ $($ODIN check ../test_issue_asm_named_register_slot.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 8 ]]; then + echo "SUCCESSFUL 1/1" + else + echo "SUCCESSFUL 0/1" + exit 1 + fi +fi $ODIN check ../test_issue_foreign_redeclaration.odin -no-entry-point $COMMON_CHECK if [[ $($ODIN check ../test_issue_foreign_redeclaration_mismatch.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]]; then echo "SUCCESSFUL 1/1" diff --git a/tests/issues/test_issue_asm_named_register_slot.odin b/tests/issues/test_issue_asm_named_register_slot.odin new file mode 100644 index 000000000..1b6362510 --- /dev/null +++ b/tests/issues/test_issue_asm_named_register_slot.odin @@ -0,0 +1,41 @@ +#+build amd64 +// A slot that only a named hardware register can fill (segment/control/debug/x87/MMX) +// carries no width and no register class, so it used to absorb any operand at all and +// hand the backend an instruction that does not encode. +package test_issues + +// Rejected: none of these widths pair up, and only `mov`'s segment-register forms ever +// admitted them. +bad_64_32 :: asm(a: i64) -> (r: i32) { mov r, a; } +bad_8_16 :: asm(a: u8) -> (r: u16) { mov r, a; } +bad_16_8 :: asm(a: u16) -> (r: u8) { mov r, a; } +bad_64_8 :: asm(a: u64) -> (r: u8) { mov r, a; } + +// Accepted: equal widths, regardless of signedness or pointer spelling. +ok_32 :: asm(a: i32) -> (r: u32) { mov r, a; } +ok_64 :: asm(a: u64) -> (r: i64) { mov r, a; } +ok_ptr :: asm(a: rawptr) -> (r: ^i32) { mov r, a; } + +// Accepted: the named registers those forms are actually for. +ok_seg :: asm(a: u64) -> (r: u64) { mov %ds, a; mov r, a; } +ok_ctrl :: asm(a: u64) -> (r: u64) { mov %cr0, a; mov r, a; } +ok_dbg :: asm(a: u64) -> (r: u64) { mov %dr0, a; mov r, a; } + +use :: proc() { + a8: u8 + a16: u16 + a32: i32 + a64: i64 + au64: u64 + ap: rawptr + _ = bad_64_32(a64) + _ = bad_8_16(a8) + _ = bad_16_8(a16) + _ = bad_64_8(au64) + _ = ok_32(a32) + _ = ok_64(au64) + _ = ok_ptr(ap) + _ = ok_seg(au64) + _ = ok_ctrl(au64) + _ = ok_dbg(au64) +}