From c56c053302f9d59ba15942af38472286050440d5 Mon Sep 17 00:00:00 2001 From: Jason Eckhardt Date: Sun, 18 Feb 2024 20:10:16 -0600 Subject: [PATCH] [TableGen] Fixes for per-HwMode decoding problem. 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. --- llvm/test/TableGen/HwModeEncodeDecode.td | 5 ++-- llvm/utils/TableGen/DecoderEmitter.cpp | 35 ++++++++++++++++-------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/llvm/test/TableGen/HwModeEncodeDecode.td b/llvm/test/TableGen/HwModeEncodeDecode.td index f8fcfafdd968f..aaacd7d3f6590 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 02d9527fd9ed9..22a71065134a4 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; + 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 &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 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 HwModeNames; const auto &NumberedInstructions = Target.getInstructionsByEnumValue(); NumberedEncodings.reserve(NumberedInstructions.size()); - DenseMap 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(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(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