[llvm] 9a26f89 - [llvm-tblgen] NFC: Simplify DecoderEmitter.
James Y Knight via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 28 16:45:27 PDT 2022
Author: James Y Knight
Date: 2022-10-28T19:45:20-04:00
New Revision: 9a26f8931657be611b0111e604610ac967aa5b62
URL: https://github.com/llvm/llvm-project/commit/9a26f8931657be611b0111e604610ac967aa5b62
DIFF: https://github.com/llvm/llvm-project/commit/9a26f8931657be611b0111e604610ac967aa5b62.diff
LOG: [llvm-tblgen] NFC: Simplify DecoderEmitter.
Currently the DecoderEmitter constructor takes a bunch of string
parameters containing bits of code to interpolate.
However, there's only two ways it can be called. The one used for most
targets which doesn't handle the SoftFail DecoderStatus (not a
problem, because they don't use SoftFail). The other mode, which is
used for ARM/AArch64, does handle SoftFail, but requires an externally
defined helper function in those targets.
This is unnecessary complication; remove the parameters, and unify
onto a single version which does support SoftFail, defining the helper
itself.
Added:
Modified:
llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
llvm/test/TableGen/VarLenDecoder.td
llvm/test/TableGen/trydecode-emission.td
llvm/test/TableGen/trydecode-emission2.td
llvm/test/TableGen/trydecode-emission3.td
llvm/utils/TableGen/DecoderEmitter.cpp
llvm/utils/TableGen/DisassemblerEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp b/llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
index 951f24998084b..b5f205f7705fc 100644
--- a/llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
+++ b/llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
@@ -271,21 +271,6 @@ static DecodeStatus DecodeSETMemOpInstruction(MCInst &Inst, uint32_t insn,
uint64_t Addr,
const MCDisassembler *Decoder);
-static bool Check(DecodeStatus &Out, DecodeStatus In) {
- switch (In) {
- case MCDisassembler::Success:
- // Out stays the same.
- return true;
- case MCDisassembler::SoftFail:
- Out = In;
- return true;
- case MCDisassembler::Fail:
- Out = In;
- return false;
- }
- llvm_unreachable("Invalid DecodeStatus!");
-}
-
#include "AArch64GenDisassemblerTables.inc"
#include "AArch64GenInstrInfo.inc"
diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index 8169be4e0b568..cb2a0c43ea421 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -165,21 +165,6 @@ class ARMDisassembler : public MCDisassembler {
} // end anonymous namespace
-static bool Check(DecodeStatus &Out, DecodeStatus In) {
- switch (In) {
- case MCDisassembler::Success:
- // Out stays the same.
- return true;
- case MCDisassembler::SoftFail:
- Out = In;
- return true;
- case MCDisassembler::Fail:
- Out = In;
- return false;
- }
- llvm_unreachable("Invalid DecodeStatus!");
-}
-
// Forward declare these because the autogenerated code will reference them.
// Definitions are further down.
static DecodeStatus DecodeGPRRegisterClass(MCInst &Inst, unsigned RegNo,
diff --git a/llvm/test/TableGen/VarLenDecoder.td b/llvm/test/TableGen/VarLenDecoder.td
index 42f81a9d0cf3f..512cae8dbf662 100644
--- a/llvm/test/TableGen/VarLenDecoder.td
+++ b/llvm/test/TableGen/VarLenDecoder.td
@@ -59,17 +59,17 @@ def FOO32 : MyVarInst<MemOp32> {
// CHECK: case 0:
// CHECK-NEXT: tmp = fieldFromInstruction(insn, 8, 3);
-// CHECK-NEXT: if (DecodeRegClassRegisterClass(MI, tmp, Address, Decoder) == MCDisassembler::Fail) { return MCDisassembler::Fail; }
+// CHECK-NEXT: if (!Check(S, DecodeRegClassRegisterClass(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; }
// CHECK-NEXT: tmp = fieldFromInstruction(insn, 0, 3);
-// CHECK-NEXT: if (DecodeRegClassRegisterClass(MI, tmp, Address, Decoder) == MCDisassembler::Fail) { return MCDisassembler::Fail; }
+// CHECK-NEXT: if (!Check(S, DecodeRegClassRegisterClass(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; }
// CHECK-NEXT: tmp = fieldFromInstruction(insn, 11, 16);
// CHECK-NEXT: MI.addOperand(MCOperand::createImm(tmp));
// CHECK-NEXT: return S;
// CHECK-NEXT: case 1:
// CHECK-NEXT: tmp = fieldFromInstruction(insn, 8, 3);
-// CHECK-NEXT: if (DecodeRegClassRegisterClass(MI, tmp, Address, Decoder) == MCDisassembler::Fail) { return MCDisassembler::Fail; }
+// CHECK-NEXT: if (!Check(S, DecodeRegClassRegisterClass(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; }
// CHECK-NEXT: tmp = fieldFromInstruction(insn, 0, 3);
-// CHECK-NEXT: if (DecodeRegClassRegisterClass(MI, tmp, Address, Decoder) == MCDisassembler::Fail) { return MCDisassembler::Fail; }
+// CHECK-NEXT: if (!Check(S, DecodeRegClassRegisterClass(MI, tmp, Address, Decoder))) { return MCDisassembler::Fail; }
// CHECK-NEXT: tmp = 0x0;
// CHECK-NEXT: insertBits(tmp, fieldFromInstruction(insn, 11, 16), 16, 16);
// CHECK-NEXT: insertBits(tmp, fieldFromInstruction(insn, 27, 16), 0, 16);
diff --git a/llvm/test/TableGen/trydecode-emission.td b/llvm/test/TableGen/trydecode-emission.td
index fb40ee1802709..dcde8a7e9770b 100644
--- a/llvm/test/TableGen/trydecode-emission.td
+++ b/llvm/test/TableGen/trydecode-emission.td
@@ -40,4 +40,4 @@ def InstB : TestInstruction {
// CHECK-NEXT: /* 22 */ MCD::OPC_Decode, {{[0-9]+}}, 1, 1, // Opcode: InstA
// CHECK-NEXT: /* 26 */ MCD::OPC_Fail,
-// CHECK: if (DecodeInstB(MI, insn, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
+// CHECK: if (!Check(S, DecodeInstB(MI, insn, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
diff --git a/llvm/test/TableGen/trydecode-emission2.td b/llvm/test/TableGen/trydecode-emission2.td
index 5011133801220..da89f9135a322 100644
--- a/llvm/test/TableGen/trydecode-emission2.td
+++ b/llvm/test/TableGen/trydecode-emission2.td
@@ -40,5 +40,5 @@ def InstB : TestInstruction {
// CHECK-NEXT: /* 37 */ MCD::OPC_TryDecode, {{[0-9]+}}, 1, 1, 0, 0, 0, // Opcode: InstA, skip to: 44
// CHECK-NEXT: /* 44 */ MCD::OPC_Fail,
-// CHECK: if (DecodeInstB(MI, insn, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
-// CHECK: if (DecodeInstA(MI, insn, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
+// CHECK: if (!Check(S, DecodeInstB(MI, insn, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
+// CHECK: if (!Check(S, DecodeInstA(MI, insn, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
diff --git a/llvm/test/TableGen/trydecode-emission3.td b/llvm/test/TableGen/trydecode-emission3.td
index 84ce4f9a749b1..17a86ae88f930 100644
--- a/llvm/test/TableGen/trydecode-emission3.td
+++ b/llvm/test/TableGen/trydecode-emission3.td
@@ -41,4 +41,4 @@ def InstB : TestInstruction {
// CHECK-NEXT: /* 22 */ MCD::OPC_Decode, {{[0-9]+}}, 1, 1, // Opcode: InstA
// CHECK-NEXT: /* 26 */ MCD::OPC_Fail,
-// CHECK: if (DecodeInstBOp(MI, tmp, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
+// CHECK: if (!Check(S, DecodeInstBOp(MI, tmp, Address, Decoder))) { DecodeComplete = false; return MCDisassembler::Fail; }
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 0e2b106279997..05d81bae0a9d3 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -127,17 +127,8 @@ class DecoderEmitter {
std::vector<EncodingAndInst> NumberedEncodings;
public:
- // Defaults preserved here for documentation, even though they aren't
- // strictly necessary given the way that this is currently being called.
- DecoderEmitter(RecordKeeper &R, std::string PredicateNamespace,
- std::string GPrefix = "if (",
- std::string GPostfix = " == MCDisassembler::Fail)",
- std::string ROK = "MCDisassembler::Success",
- std::string RFail = "MCDisassembler::Fail", std::string L = "")
- : RK(R), Target(R), PredicateNamespace(std::move(PredicateNamespace)),
- GuardPrefix(std::move(GPrefix)), GuardPostfix(std::move(GPostfix)),
- ReturnOK(std::move(ROK)), ReturnFail(std::move(RFail)),
- Locals(std::move(L)) {}
+ DecoderEmitter(RecordKeeper &R, std::string PredicateNamespace)
+ : RK(R), Target(R), PredicateNamespace(std::move(PredicateNamespace)) {}
// Emit the decoder state machine table.
void emitTable(formatted_raw_ostream &o, DecoderTable &Table,
@@ -160,9 +151,6 @@ class DecoderEmitter {
public:
std::string PredicateNamespace;
- std::string GuardPrefix, GuardPostfix;
- std::string ReturnOK, ReturnFail;
- std::string Locals;
};
} // end anonymous namespace
@@ -1170,11 +1158,11 @@ void FilterChooser::emitBinaryParser(raw_ostream &o, unsigned &Indentation,
if (Decoder != "") {
OpHasCompleteDecoder = OpInfo.HasCompleteDecoder;
- o.indent(Indentation) << Emitter->GuardPrefix << Decoder
- << "(MI, tmp, Address, Decoder)"
- << Emitter->GuardPostfix
- << " { " << (OpHasCompleteDecoder ? "" : "DecodeComplete = false; ")
- << "return MCDisassembler::Fail; }\n";
+ o.indent(Indentation) << "if (!Check(S, " << Decoder
+ << "(MI, tmp, Address, Decoder))) { "
+ << (OpHasCompleteDecoder ? ""
+ : "DecodeComplete = false; ")
+ << "return MCDisassembler::Fail; }\n";
} else {
OpHasCompleteDecoder = true;
o.indent(Indentation) << "MI.addOperand(MCOperand::createImm(tmp));\n";
@@ -1189,11 +1177,11 @@ void FilterChooser::emitDecoder(raw_ostream &OS, unsigned Indentation,
// If a custom instruction decoder was specified, use that.
if (Op.numFields() == 0 && !Op.Decoder.empty()) {
HasCompleteDecoder = Op.HasCompleteDecoder;
- OS.indent(Indentation) << Emitter->GuardPrefix << Op.Decoder
- << "(MI, insn, Address, Decoder)"
- << Emitter->GuardPostfix
- << " { " << (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
- << "return MCDisassembler::Fail; }\n";
+ OS.indent(Indentation)
+ << "if (!Check(S, " << Op.Decoder
+ << "(MI, insn, Address, Decoder))) { "
+ << (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
+ << "return MCDisassembler::Fail; }\n";
break;
}
@@ -2529,6 +2517,17 @@ static void emitDecodeInstruction(formatted_raw_ostream &OS,
<< "}\n\n";
}
+// Helper to propagate SoftFail status. Returns false if the status is Fail;
+// callers are expected to early-exit in that condition. (Note, the '&' operator
+// is correct to propagate the values of this enum; see comment on 'enum
+// DecodeStatus'.)
+static void emitCheck(formatted_raw_ostream &OS) {
+ OS << "static bool Check(DecodeStatus &Out, DecodeStatus In) {\n"
+ << " Out = static_cast<DecodeStatus>(Out & In);"
+ << " return Out != MCDisassembler::Fail;\n"
+ << "}\n\n";
+}
+
// Emits disassembler code for instruction decoding.
void DecoderEmitter::run(raw_ostream &o) {
formatted_raw_ostream OS(o);
@@ -2545,6 +2544,7 @@ void DecoderEmitter::run(raw_ostream &o) {
emitFieldFromInstruction(OS);
emitInsertBits(OS);
+ emitCheck(OS);
Target.reverseBitsForLittleEndianEncoding();
@@ -2703,12 +2703,8 @@ void DecoderEmitter::run(raw_ostream &o) {
namespace llvm {
void EmitDecoder(RecordKeeper &RK, raw_ostream &OS,
- const std::string &PredicateNamespace,
- const std::string &GPrefix, const std::string &GPostfix,
- const std::string &ROK, const std::string &RFail,
- const std::string &L) {
- DecoderEmitter(RK, PredicateNamespace, GPrefix, GPostfix, ROK, RFail, L)
- .run(OS);
+ const std::string &PredicateNamespace) {
+ DecoderEmitter(RK, PredicateNamespace).run(OS);
}
} // end namespace llvm
diff --git a/llvm/utils/TableGen/DisassemblerEmitter.cpp b/llvm/utils/TableGen/DisassemblerEmitter.cpp
index 297d12c5d0e92..dfa4b30ee569e 100644
--- a/llvm/utils/TableGen/DisassemblerEmitter.cpp
+++ b/llvm/utils/TableGen/DisassemblerEmitter.cpp
@@ -96,10 +96,7 @@ using namespace llvm::X86Disassembler;
namespace llvm {
extern void EmitDecoder(RecordKeeper &RK, raw_ostream &OS,
- const std::string &PredicateNamespace,
- const std::string &GPrefix, const std::string &GPostfix,
- const std::string &ROK, const std::string &RFail,
- const std::string &L);
+ const std::string &PredicateNamespace);
void EmitDisassembler(RecordKeeper &Records, raw_ostream &OS) {
CodeGenTarget Target(Records);
@@ -132,23 +129,10 @@ void EmitDisassembler(RecordKeeper &Records, raw_ostream &OS) {
return;
}
- // ARM and Thumb have a CHECK() macro to deal with DecodeStatuses.
- if (Target.getName() == "ARM" || Target.getName() == "Thumb" ||
- Target.getName() == "AArch64" || Target.getName() == "ARM64") {
- std::string PredicateNamespace = std::string(Target.getName());
- if (PredicateNamespace == "Thumb")
- PredicateNamespace = "ARM";
-
- EmitDecoder(Records, OS, PredicateNamespace, "if (!Check(S, ", "))", "S",
- "MCDisassembler::Fail",
- " MCDisassembler::DecodeStatus S = "
- "MCDisassembler::Success;\n(void)S;");
- return;
- }
-
- EmitDecoder(Records, OS, std::string(Target.getName()), "if (",
- " == MCDisassembler::Fail)", "MCDisassembler::Success",
- "MCDisassembler::Fail", "");
+ std::string PredicateNamespace = std::string(Target.getName());
+ if (PredicateNamespace == "Thumb")
+ PredicateNamespace = "ARM";
+ EmitDecoder(Records, OS, PredicateNamespace);
}
} // end namespace llvm
More information about the llvm-commits
mailing list