Automerge: [TableGen][DecoderEmitter] Infer encoding's HasCompleteDecoder earlier (NFCI) (#154644)

If an encoding has a custom decoder, the decoder is assumed to be
"complete" (always succeed) if hasCompleteDecoder field is true. We
determine this when constructing InstructionEncoding.

If the decoder for an encoding is *generated*, it always succeeds if
none of the operand decoders can fail. The latter is determined based on
the value of operands' DecoderMethod/hasCompleteDecoder. This happens
late, at table construction time, making the code harder to follow.

This change moves this logic to the InstructionEncoding constructor.

Author: s-barannikov

llvm/utils/TableGen/DecoderEmitter.cpp

@@ -147,9 +147,10 @@ class InstructionEncoding {
   /// 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;
+  /// Whether the custom decoding function always succeeds. If a custom decoder
+  /// function is specified, the value is taken from the target description,
+  /// otherwise it is inferred.
+  bool HasCompleteDecoder;
 
   /// Information about the operands' contribution to this encoding.
   SmallVector<OperandInfo, 16> Operands;
@@ -174,11 +175,9 @@ class InstructionEncoding {
   /// 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 whether the decoder (either generated or specified by the user)
+  /// always succeeds.
+  bool hasCompleteDecoder() const { return HasCompleteDecoder; }
 
   /// Returns information about the operands' contribution to this encoding.
   ArrayRef<OperandInfo> getOperands() const { return Operands; }
@@ -658,12 +657,11 @@ class FilterChooser {
   void emitSingletonTableEntry(DecoderTableInfo &TableInfo,
                                const Filter &Best) const;
 
-  bool emitBinaryParser(raw_ostream &OS, indent Indent,
+  void emitBinaryParser(raw_ostream &OS, indent Indent,
                         const OperandInfo &OpInfo) const;
 
-  bool emitDecoder(raw_ostream &OS, indent Indent, unsigned EncodingID) const;
-  std::pair<unsigned, bool> getDecoderIndex(DecoderSet &Decoders,
-                                            unsigned EncodingID) const;
+  void emitDecoder(raw_ostream &OS, indent Indent, unsigned EncodingID) const;
+  unsigned getDecoderIndex(DecoderSet &Decoders, unsigned EncodingID) const;
 
   // Assign a single filter and run with it.
   void runSingleFilter(unsigned startBit, unsigned numBit);
@@ -1212,7 +1210,7 @@ FilterChooser::getIslands(const insn_t &Insn) const {
   return Islands;
 }
 
-bool FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
+void FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
                                      const OperandInfo &OpInfo) const {
   const std::string &Decoder = OpInfo.Decoder;
 
@@ -1238,65 +1236,56 @@ bool FilterChooser::emitBinaryParser(raw_ostream &OS, indent Indent,
     OS << ";\n";
   }
 
-  bool OpHasCompleteDecoder;
   if (!Decoder.empty()) {
-    OpHasCompleteDecoder = OpInfo.HasCompleteDecoder;
     OS << Indent << "if (!Check(S, " << Decoder
        << "(MI, tmp, Address, Decoder))) { "
-       << (OpHasCompleteDecoder ? "" : "DecodeComplete = false; ")
+       << (OpInfo.HasCompleteDecoder ? "" : "DecodeComplete = false; ")
        << "return MCDisassembler::Fail; }\n";
   } else {
-    OpHasCompleteDecoder = true;
     OS << Indent << "MI.addOperand(MCOperand::createImm(tmp));\n";
   }
-  return OpHasCompleteDecoder;
 }
 
-bool FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
+void FilterChooser::emitDecoder(raw_ostream &OS, indent Indent,
                                 unsigned EncodingID) const {
   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; ")
+       << (Encoding.hasCompleteDecoder() ? "" : "DecodeComplete = false; ")
        << "return MCDisassembler::Fail; }\n";
-    return HasCompleteDecoder;
+    return;
   }
 
-  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
          << "(MI, insn, Address, Decoder))) { "
-         << (HasCompleteDecoder ? "" : "DecodeComplete = false; ")
+         << (Op.HasCompleteDecoder ? "" : "DecodeComplete = false; ")
          << "return MCDisassembler::Fail; }\n";
       break;
     }
 
-    HasCompleteDecoder &= emitBinaryParser(OS, Indent, Op);
+    emitBinaryParser(OS, Indent, Op);
   }
-  return HasCompleteDecoder;
 }
 
-std::pair<unsigned, bool>
-FilterChooser::getDecoderIndex(DecoderSet &Decoders,
-                               unsigned EncodingID) const {
+unsigned FilterChooser::getDecoderIndex(DecoderSet &Decoders,
+                                        unsigned EncodingID) const {
   // Build up the predicate string.
   SmallString<256> Decoder;
   // FIXME: emitDecoder() function can take a buffer directly rather than
   // a stream.
   raw_svector_ostream S(Decoder);
   indent Indent(UseFnTableInDecodeToMCInst ? 2 : 4);
-  bool HasCompleteDecoder = emitDecoder(S, Indent, EncodingID);
+  emitDecoder(S, Indent, EncodingID);
 
   // Using the full decoder string as the key value here is a bit
   // heavyweight, but is effective. If the string comparisons become a
@@ -1308,7 +1297,7 @@ FilterChooser::getDecoderIndex(DecoderSet &Decoders,
   Decoders.insert(CachedHashString(Decoder));
   // Now figure out the index for when we write out the table.
   DecoderSet::const_iterator P = find(Decoders, Decoder.str());
-  return {(unsigned)(P - Decoders.begin()), HasCompleteDecoder};
+  return std::distance(Decoders.begin(), P);
 }
 
 // If ParenIfBinOp is true, print a surrounding () if Val uses && or ||.
@@ -1508,8 +1497,7 @@ void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo,
   // Check for soft failure of the match.
   emitSoftFailTableEntry(TableInfo, EncodingID);
 
-  auto [DIdx, HasCompleteDecoder] =
-      getDecoderIndex(TableInfo.Decoders, EncodingID);
+  unsigned DIdx = getDecoderIndex(TableInfo.Decoders, EncodingID);
 
   // Produce OPC_Decode or OPC_TryDecode opcode based on the information
   // whether the instruction decoder is complete or not. If it is complete
@@ -1520,10 +1508,12 @@ void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo,
   // decoder method indicates that additional processing should be done to see
   // if there is any other instruction that also matches the bitpattern and
   // can decode it.
-  const uint8_t DecoderOp = HasCompleteDecoder ? MCD::OPC_Decode
-                                               : (TableInfo.isOutermostScope()
-                                                      ? MCD::OPC_TryDecodeOrFail
-                                                      : MCD::OPC_TryDecode);
+  const InstructionEncoding &Encoding = Encodings[EncodingID];
+  const uint8_t DecoderOp =
+      Encoding.hasCompleteDecoder()
+          ? MCD::OPC_Decode
+          : (TableInfo.isOutermostScope() ? MCD::OPC_TryDecodeOrFail
+                                          : MCD::OPC_TryDecode);
   TableInfo.Table.push_back(DecoderOp);
   const Record *InstDef = Encodings[EncodingID].getInstruction()->TheDef;
   TableInfo.Table.insertULEB128(Emitter->getTarget().getInstrIntValue(InstDef));
@@ -2124,6 +2114,16 @@ InstructionEncoding::InstructionEncoding(const Record *EncodingDef,
     if (DecoderMethod.empty())
       parseFixedLenOperands(*BI);
   }
+
+  if (DecoderMethod.empty()) {
+    // A generated decoder is always successful if none of the operand
+    // decoders can fail (all are always successful).
+    HasCompleteDecoder = all_of(Operands, [](const OperandInfo &Op) {
+      // By default, a generated operand decoder is assumed to always succeed.
+      // This can be overridden by the user.
+      return Op.Decoder.empty() || Op.HasCompleteDecoder;
+    });
+  }
 }
 
 // emitFieldFromInstruction - Emit the templated helper function