[llvm] 2ed0aac - [TableGen] Fixes for per-HwMode decoding problem (#82201)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Feb 18 21:14:26 PST 2024
Author: Jason Eckhardt
Date: 2024-02-19T13:14:22+08:00
New Revision: 2ed0aacf973d628d5abae79229e251e034831dd3
URL: https://github.com/llvm/llvm-project/commit/2ed0aacf973d628d5abae79229e251e034831dd3
DIFF: https://github.com/llvm/llvm-project/commit/2ed0aacf973d628d5abae79229e251e034831dd3.diff
LOG: [TableGen] Fixes for per-HwMode decoding problem (#82201)
Today, if any instruction uses EncodingInfos/EncodingByHwMode to
override the default encoding, the opcode field of the decoder table is
generated incorrectly. This causes failed disassemblies and other
problems.
Specifically, the main correctness issue is that the EncodingID is
inadvertently stored in the table rather than the actual opcode. This is
caused by having set up the IndexOfInstruction map incorrectly during
the loop to populate NumberedEncodings-- which is then propagated around
when OpcMap is set up with a bad EncodingIDAndOpcode.
Instead, do away with IndexOfInstruction altogether and use opcode value
queried from CodeGenTarget::getInstrIntValue to set up OpcMap. This
itself exposed another problem where emitTable was using the decoded
opcode to index into NumberedEncodings. Instead pass in the
EncodingIDAndOpcode vector, and create the reverse mapping from Opcode
to EncodingID, which is then used to index NumberedEncodings.
This problem is not currently exposed upstream since no in-tree targets
yet use the per-HwMode feature. It does show up in at least two
downstream targets.
Added:
Modified:
llvm/test/TableGen/HwModeEncodeDecode.td
llvm/utils/TableGen/DecoderEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/test/TableGen/HwModeEncodeDecode.td b/llvm/test/TableGen/HwModeEncodeDecode.td
index f8fcfafdd968fa..aaacd7d3f6590a 100644
--- a/llvm/test/TableGen/HwModeEncodeDecode.td
+++ b/llvm/test/TableGen/HwModeEncodeDecode.td
@@ -74,9 +74,8 @@ def baz : Instruction {
// DECODER-DAG: Opcode: fooTypeEncA:foo
// DECODER-DAG: Opcode: bar
// DECODER-LABEL: DecoderTable_ModeB32[] =
-// Note that the comment says fooTypeEncA but this is actually fooTypeEncB; plumbing
-// the correct comment text through the decoder is nontrivial.
-// DECODER-DAG: Opcode: fooTypeEncA:foo
+// DECODER-DAG: Opcode: fooTypeEncB:foo
+// DECODER-DAG: Opcode: fooTypeEncA:baz
// DECODER-DAG: Opcode: bar
// ENCODER-LABEL: static const uint64_t InstBits_ModeA[] = {
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 02d9527fd9ed91..22a71065134a4e 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -118,6 +118,8 @@ struct EncodingIDAndOpcode {
: EncodingID(EncodingID), Opcode(Opcode) {}
};
+using EncodingIDsVec = std::vector<EncodingIDAndOpcode>;
+
raw_ostream &operator<<(raw_ostream &OS, const EncodingAndInst &Value) {
if (Value.EncodingDef != Value.Inst->TheDef)
OS << Value.EncodingDef->getName() << ":";
@@ -135,8 +137,8 @@ class DecoderEmitter {
// Emit the decoder state machine table.
void emitTable(formatted_raw_ostream &o, DecoderTable &Table,
- unsigned Indentation, unsigned BitWidth,
- StringRef Namespace) const;
+ unsigned Indentation, unsigned BitWidth, StringRef Namespace,
+ const EncodingIDsVec &EncodingIDs) const;
void emitInstrLenTable(formatted_raw_ostream &OS,
std::vector<unsigned> &InstrLen) const;
void emitPredicateFunction(formatted_raw_ostream &OS,
@@ -766,7 +768,16 @@ unsigned Filter::usefulness() const {
// Emit the decoder state machine table.
void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
unsigned Indentation, unsigned BitWidth,
- StringRef Namespace) const {
+ StringRef Namespace,
+ const EncodingIDsVec &EncodingIDs) const {
+ // We'll need to be able to map from a decoded opcode into the corresponding
+ // EncodingID for this specific combination of BitWidth and Namespace. This
+ // is used below to index into NumberedEncodings.
+ DenseMap<unsigned, unsigned> OpcodeToEncodingID;
+ OpcodeToEncodingID.reserve(EncodingIDs.size());
+ for (auto &EI : EncodingIDs)
+ OpcodeToEncodingID[EI.Opcode] = EI.EncodingID;
+
OS.indent(Indentation) << "static const uint8_t DecoderTable" << Namespace
<< BitWidth << "[] = {\n";
@@ -888,8 +899,12 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
// Decoder index.
I += emitULEB128(I, OS);
+ auto EncI = OpcodeToEncodingID.find(Opc);
+ assert(EncI != OpcodeToEncodingID.end() && "no encoding entry");
+ auto EncodingID = EncI->second;
+
if (!IsTry) {
- OS << "// Opcode: " << NumberedEncodings[Opc] << "\n";
+ OS << "// Opcode: " << NumberedEncodings[EncodingID] << "\n";
break;
}
@@ -899,7 +914,7 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
uint32_t NumToSkip = emitNumToSkip(I, OS);
I += 3;
- OS << "// Opcode: " << NumberedEncodings[Opc]
+ OS << "// Opcode: " << NumberedEncodings[EncodingID]
<< ", skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
break;
}
@@ -2449,11 +2464,8 @@ void DecoderEmitter::run(raw_ostream &o) {
std::set<StringRef> HwModeNames;
const auto &NumberedInstructions = Target.getInstructionsByEnumValue();
NumberedEncodings.reserve(NumberedInstructions.size());
- DenseMap<Record *, unsigned> IndexOfInstruction;
// First, collect all HwModes referenced by the target.
for (const auto &NumberedInstruction : NumberedInstructions) {
- IndexOfInstruction[NumberedInstruction->TheDef] = NumberedEncodings.size();
-
if (const RecordVal *RV =
NumberedInstruction->TheDef->getValue("EncodingInfos")) {
if (auto *DI = dyn_cast_or_null<DefInit>(RV->getValue())) {
@@ -2470,8 +2482,6 @@ void DecoderEmitter::run(raw_ostream &o) {
HwModeNames.insert("");
for (const auto &NumberedInstruction : NumberedInstructions) {
- IndexOfInstruction[NumberedInstruction->TheDef] = NumberedEncodings.size();
-
if (const RecordVal *RV =
NumberedInstruction->TheDef->getValue("EncodingInfos")) {
if (DefInit *DI = dyn_cast_or_null<DefInit>(RV->getValue())) {
@@ -2544,7 +2554,7 @@ void DecoderEmitter::run(raw_ostream &o) {
DecoderNamespace +=
std::string("_") + NumberedEncodings[i].HwModeName.str();
OpcMap[std::pair(DecoderNamespace, Size)].emplace_back(
- i, IndexOfInstruction.find(Def)->second);
+ i, Target.getInstrIntValue(Def));
} else {
NumEncodingsOmitted++;
}
@@ -2577,7 +2587,8 @@ void DecoderEmitter::run(raw_ostream &o) {
TableInfo.Table.push_back(MCD::OPC_Fail);
// Print the table to the output stream.
- emitTable(OS, TableInfo.Table, 0, FC.getBitWidth(), Opc.first.first);
+ emitTable(OS, TableInfo.Table, 0, FC.getBitWidth(), Opc.first.first,
+ Opc.second);
}
// For variable instruction, we emit a instruction length table
More information about the llvm-commits
mailing list