[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.

Author: nvjle

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[] = {

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