[TableGen][DecoderEmitter] Add DecoderMethod to InstructionEncoding (NFC)

[s-barannikov]

We used to abuse Operands list to store instruction encoding's DecoderMethod there.
Let's store it in the InstructionEncoding class instead, where it belongs.

[s-barannikov]

This actually helped to detect two bugs in M68k. One is fixed by #154481, the other is more difficult to fix (crash on attempt to disassemble instructions reading/writing SR or CCR).

I'll fix the FIXME in a separate PR to keep this as NFC.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

We used to abuse Operands list to store instruction encoding's DecoderMethod there.
Let's store it in the InstructionEncoding class instead, where it belongs.

---

Full diff: https://github.com/llvm/llvm-project/pull/154477.diff

1 Files Affected:

• (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+39-9)

diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 1924cf8a3adcd..f8f84a900d0dd 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -143,6 +143,14 @@ class InstructionEncoding {
   /// The size of this encoding, in bits.
   unsigned BitWidth;
 
+  /// The name of the function to use for decoding. May be an empty string,
+  /// meaning the decoder is generated.
+  StringRef DecoderMethod;
+
+  /// Whether the custom decoding function always succeeds. Should not be used
+  /// if the decoder is generated.
+  bool HasCompleteDecoder = true;
+
   /// Information about the operands' contribution to this encoding.
   SmallVector<OperandInfo, 16> Operands;
 
@@ -171,6 +179,16 @@ class InstructionEncoding {
   /// Returns the size of this encoding, in bits.
   unsigned getBitWidth() const { return BitWidth; }
 
+  /// Returns the name of the function to use for decoding, or an empty string
+  /// if the decoder is generated.
+  StringRef getDecoderMethod() const { return DecoderMethod; }
+
+  /// Returns whether the custom decoding function always succeeds.
+  bool hasCompleteDecoder() const {
+    assert(!DecoderMethod.empty());
+    return HasCompleteDecoder;
+  }
+
   /// Returns information about the operands' contribution to this encoding.
   ArrayRef<OperandInfo> getOperands() const { return Operands; }
 
@@ -1246,10 +1264,25 @@ bool FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
 
 bool FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
                                 unsigned EncodingID) const {
-  bool HasCompleteDecoder = true;
+  const InstructionEncoding &Encoding = Encodings[EncodingID];
+
+  // If a custom instruction decoder was specified, use that.
+  StringRef DecoderMethod = Encoding.getDecoderMethod();
+  if (!DecoderMethod.empty()) {
+    bool HasCompleteDecoder = Encoding.hasCompleteDecoder();
+    OS << Indent << "if (!Check(S, " << DecoderMethod
+       << "(MI, insn, Address, Decoder))) { "
+       << (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
+       << "return MCDisassembler::Fail; }\n";
+    return HasCompleteDecoder;
+  }
 
-  for (const OperandInfo &Op : Encodings[EncodingID].getOperands()) {
-    // If a custom instruction decoder was specified, use that.
+  bool HasCompleteDecoder = true;
+  for (const OperandInfo &Op : Encoding.getOperands()) {
+    // FIXME: This is broken. If there is an operand that doesn't contribute
+    //   to the encoding, we generate the same code as if the decoder method
+    //   was specified on the encoding. And then we stop, ignoring the rest
+    //   of the operands. M68k disassembler experiences this.
     if (Op.numFields() == 0 && !Op.Decoder.empty()) {
       HasCompleteDecoder = Op.HasCompleteDecoder;
       OS << Indent << "if (!Check(S, " << Op.Decoder
@@ -1258,7 +1291,6 @@ bool FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
          << "return MCDisassembler::Fail; }\n";
       break;
     }
-
     HasCompleteDecoder &= emitBinaryParser(OS, Indent, Op);
   }
   return HasCompleteDecoder;
@@ -1971,11 +2003,9 @@ unsigned InstructionEncoding::populateEncoding() {
 
   // If the instruction has specified a custom decoding hook, use that instead
   // of trying to auto-generate the decoder.
-  StringRef InstDecoder = EncodingDef->getValueAsString("DecoderMethod");
-  if (!InstDecoder.empty()) {
-    bool HasCompleteInstDecoder =
-        EncodingDef->getValueAsBit("hasCompleteDecoder");
-    Operands.push_back(OperandInfo(InstDecoder.str(), HasCompleteInstDecoder));
+  DecoderMethod = EncodingDef->getValueAsString("DecoderMethod");
+  if (!DecoderMethod.empty()) {
+    HasCompleteDecoder = EncodingDef->getValueAsBit("hasCompleteDecoder");
     return Bits.getNumBits();
   }
 

[topperc]

LGTM