From 21ad041232ec7766bb24f7777cb533c881ba3159 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Tue, 11 Aug 2026 17:44:55 +0100 Subject: [PATCH] Handle prefixes better --- .../x86/tablegen/cpp-compiler/cpp-gen.odin | 57 +++++++++++++++++++ src/asm_tables_amd64.cpp | 42 ++++++++++++++ src/check_asm.cpp | 48 ++++++++++++---- 3 files changed, 137 insertions(+), 10 deletions(-) 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 c658df2ca..a5705dde6 100644 --- a/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin +++ b/core/rexcode/isa/x86/tablegen/cpp-compiler/cpp-gen.odin @@ -82,6 +82,12 @@ main :: proc() { strings.write_string(&sb, "\tstatic String const mnemonic_strings[MNEMONIC_COUNT];\n") strings.write_string(&sb, "\tstatic String const prefix_strings[PREFIX_COUNT];\n") + { + strings.write_string(&sb, "\n"); + strings.write_string(&sb, "\tenum PrefixKind : u8 { PrefixKind_None, PrefixKind_Lock, PrefixKind_Rep, PrefixKind_Repne, PrefixKind_Other };\n"); + strings.write_string(&sb, "\n"); + } + { strings.write_string(&sb, "\n"); strings.write_string(&sb, "\t// Register classes (upper byte)\n") @@ -186,6 +192,18 @@ main :: proc() { fmt.sbprintf(&sb, "return cast(u8)((flags>>%du)&((1u<<%d)-1));", bit_offset, bit_size) strings.write_string(&sb, " }\n") } + { + bit_offset := intrinsics.type_field_bit_offset(Encoding_Flags, "lock_ok") + strings.write_string(&sb, "\t\tbool lock_ok () const { ") + fmt.sbprintf(&sb, "return ((flags>>%du)&1) != 0;", bit_offset) + strings.write_string(&sb, " }\n") + } + { + bit_offset := intrinsics.type_field_bit_offset(Encoding_Flags, "rep_ok") + strings.write_string(&sb, "\t\tbool rep_ok () const { ") + fmt.sbprintf(&sb, "return ((flags>>%du)&1) != 0;", bit_offset) + strings.write_string(&sb, " }\n") + } } strings.write_string(&sb, "\n\n") { @@ -393,6 +411,45 @@ main :: proc() { strings.write_string(&sb, "\t}\n") } + strings.write_string(&sb, "\n") + { + strings.write_string(&sb, "\tbool prefix_kind_okay(u8 prefix, Encoding const &form, bool *requires_memory_dest_) {\n") + strings.write_string(&sb, "\t\tPrefixKind kind = PrefixKind_None;\n") + strings.write_string(&sb, "\t\tif (prefix != 0) {\n") + strings.write_string(&sb, "\t\t\tswitch (prefix) {\n") + strings.write_string(&sb, "\t\t\tcase PREFIX_LOCK: kind = PrefixKind_Lock; break;\n") + strings.write_string(&sb, "\t\t\tcase PREFIX_REP: kind = PrefixKind_Rep; break;\n") + strings.write_string(&sb, "\t\t\tcase PREFIX_REPNE: kind = PrefixKind_Repne; break;\n") + strings.write_string(&sb, "\t\t\tdefault: kind = PrefixKind_Other; break;\n") + strings.write_string(&sb, "\t\t\t}\n") + strings.write_string(&sb, "\t\t}\n") + strings.write_string(&sb, "\t\tswitch (kind) {\n") + strings.write_string(&sb, "\t\tcase PrefixKind_Lock:\n") + strings.write_string(&sb, "\t\t\tif (!form.lock_ok()) {\n") + strings.write_string(&sb, "\t\t\t\treturn false;\n") + strings.write_string(&sb, "\t\t\t} else {\n") + strings.write_string(&sb, "\t\t\t\tif (requires_memory_dest_) *requires_memory_dest_ = true;\n") + strings.write_string(&sb, "\t\t\t\treturn true;\n") + strings.write_string(&sb, "\t\t\t}\n") + strings.write_string(&sb, "\t\tcase PrefixKind_Rep:\n") + strings.write_string(&sb, "\t\t\tif (!form.rep_ok()) {\n") + strings.write_string(&sb, "\t\t\t\treturn false;\n") + strings.write_string(&sb, "\t\t\t} else {\n") + strings.write_string(&sb, "\t\t\t\treturn true;\n") + strings.write_string(&sb, "\t\t\t}\n") + strings.write_string(&sb, "\t\tcase PrefixKind_Repne:\n") + strings.write_string(&sb, "\t\t\tif (!form.rep_ok()) {\n") + strings.write_string(&sb, "\t\t\t\treturn false;\n") + strings.write_string(&sb, "\t\t\t} else {\n") + strings.write_string(&sb, "\t\t\t\treturn true;\n") + strings.write_string(&sb, "\t\t\t}\n") + strings.write_string(&sb, "\t\tcase PrefixKind_Other:\n") + strings.write_string(&sb, "\t\tcase PrefixKind_None:\n") + strings.write_string(&sb, "\t\t\treturn true;\n") + strings.write_string(&sb, "\t\t}\n") + strings.write_string(&sb, "\t\treturn true;\n") + strings.write_string(&sb, "\t}\n") + } strings.write_string(&sb, "};\n") diff --git a/src/asm_tables_amd64.cpp b/src/asm_tables_amd64.cpp index 1a451c124..2a4e6010d 100644 --- a/src/asm_tables_amd64.cpp +++ b/src/asm_tables_amd64.cpp @@ -97,6 +97,9 @@ struct Asm_amd64 { static String const mnemonic_strings[MNEMONIC_COUNT]; static String const prefix_strings[PREFIX_COUNT]; + enum PrefixKind : u8 { PrefixKind_None, PrefixKind_Lock, PrefixKind_Rep, PrefixKind_Repne, PrefixKind_Other }; + + // Register classes (upper byte) static const u16 REG_CLASS_NONE = 0x000; static const u16 REG_CLASS_GPR64 = 0x100; @@ -232,6 +235,8 @@ struct Asm_amd64 { bool has_implicit () const { return ((flags>>21u)&1) != 0; } u8 explicit_count() const { return cast(u8)((flags>>18u)&((1u<<3)-1)); } u8 op_count () const { return cast(u8)((flags>>22u)&((1u<<3)-1)); } + bool lock_ok () const { return ((flags>>14u)&1) != 0; } + bool rep_ok () const { return ((flags>>15u)&1) != 0; } }; #pragma pack(pop) GB_STATIC_ASSERT(gb_size_of(Encoding) == 16); @@ -408,6 +413,43 @@ struct Asm_amd64 { } return -1; } + + bool prefix_kind_okay(u8 prefix, Encoding const &form, bool *requires_memory_dest_) { + PrefixKind kind = PrefixKind_None; + if (prefix != 0) { + switch (prefix) { + case PREFIX_LOCK: kind = PrefixKind_Lock; break; + case PREFIX_REP: kind = PrefixKind_Rep; break; + case PREFIX_REPNE: kind = PrefixKind_Repne; break; + default: kind = PrefixKind_Other; break; + } + } + switch (kind) { + case PrefixKind_Lock: + if (!form.lock_ok()) { + return false; + } else { + if (requires_memory_dest_) *requires_memory_dest_ = true; + return true; + } + case PrefixKind_Rep: + if (!form.rep_ok()) { + return false; + } else { + return true; + } + case PrefixKind_Repne: + if (!form.rep_ok()) { + return false; + } else { + return true; + } + case PrefixKind_Other: + case PrefixKind_None: + return true; + } + return true; + } }; diff --git a/src/check_asm.cpp b/src/check_asm.cpp index cbeba1268..add1757c1 100644 --- a/src/check_asm.cpp +++ b/src/check_asm.cpp @@ -627,7 +627,7 @@ gb_internal AsmOperandKind determine_asm_operand_kind(Operand const *operand) { template -gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, AstAsmInstruction *instr, u16 mnemonic, Slice const &operands, u8 previous_prefix) { +gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, AstAsmInstruction *instr, u16 mnemonic, Slice const &operands, u8 previous_prefix, Ast *previous_prefix_instr) { GB_ASSERT(mnemonic > 0); auto forms = asm_ctx->encoding_forms(mnemonic); String name = asm_ctx->mnemonic_strings[mnemonic]; @@ -738,9 +738,19 @@ gb_internal void check_mnemonic(AsmCtx *asm_ctx, CheckerContext *ctx, AstAsmInst } if (matched) { if (valid_form_index >= 0 && previous_prefix > 0) { - // TODO(bill): validate the prefix for the selected form - } + auto &form = forms[valid_form_index]; + bool requires_memory_dest = false; + bool ok = asm_ctx->prefix_kind_okay(previous_prefix, form, &requires_memory_dest); + if (ok) { + if (operands.count != 0 && determine_asm_operand_kind(&operands[0]) != AsmOperand_Memory) { + error(previous_prefix_instr ? previous_prefix_instr : instr->name, + "Asm prefix requires '%.*s' to have a memory destination operand", LIT(name)); + } + } else { + error(instr->name, "Asm prefix cannot be applied to '%.*s'", LIT(name)); + } + } return; } @@ -1182,9 +1192,10 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity array_reserve(&operands, 16); defer (array_free(&operands)); - for (Ast *instruction_ : at->instructions) { - u8 previous_prefix = 0; + u8 previous_prefix = 0; + Ast *previous_prefix_instr = nullptr; // for a good error location + for (Ast *instruction_ : at->instructions) { switch (instruction_->kind) { case_ast_node(instr, AsmInstruction, instruction_); GB_ASSERT(instr->name->kind == Ast_Ident); @@ -1192,30 +1203,47 @@ gb_internal void check_asm_template(AsmCtx *asm_ctx, CheckerContext *ctx, Entity u16 mnemonic = 0; CheckMnemomicResult res = check_mnemonic_name(asm_ctx, instr, &mnemonic); - array_clear(&operands); - for (Ast *expr : instr->operands) { Operand operand = {}; check_asm_instruction_operand(asm_ctx, ctx, entity, &operand, expr, /*allow_memory_operands*/true); array_add(&operands, operand); } + if (res == CheckMnemomic_Prefix) { if (instr->operands.count != 0) { error(instr->name, "A prefix must not have any operands, and be separate from the instruction it is prefixing"); } + if (previous_prefix != 0) { + error(instr->name, "A prefix cannot immediately follow another prefix"); + } previous_prefix = cast(u8)mnemonic; + previous_prefix_instr = instruction_; } else if (res == CheckMnemomic_Mnemonic) { - check_mnemonic(asm_ctx, ctx, instr, mnemonic, slice_from_array(operands), previous_prefix); + check_mnemonic(asm_ctx, ctx, instr, mnemonic, slice_from_array(operands), previous_prefix, previous_prefix_instr); + previous_prefix = 0; + previous_prefix_instr = nullptr; + } else { + // invalid mnemonic already reported; a pending prefix now has no target + previous_prefix = 0; + previous_prefix_instr = nullptr; } + case_end; - case_end; case_ast_node(label, AsmLabelDecl, instruction_); - // already done + if (previous_prefix != 0) { + error(previous_prefix_instr, "A prefix must be immediately followed by an instruction, but a label declaration was found"); + previous_prefix = 0; + previous_prefix_instr = nullptr; + } case_end; + default: error(instruction_, "Unexpected instruction in asm template"); break; } } + if (previous_prefix != 0) { + error(previous_prefix_instr, "A prefix must be immediately followed by an instruction, but the template ended"); + } } \ No newline at end of file