Skip to content

Conversation

@s-barannikov
Copy link
Contributor

@s-barannikov s-barannikov commented Aug 17, 2025

Parse the Inst and SoftField fields once and store them in InstructionEncoding so that we don't parse them every time getMandatoryEncodingBits() is called.

@llvmbot
Copy link
Member

llvmbot commented Aug 17, 2025

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

insnWithID is called multiple times for the same encoding ID. Call it once and store the result in NumberedEncodings. Also, give the method a more meaningful name.


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

1 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+38-27)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 11d06d1613e89..64149a3841573 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -209,6 +209,7 @@ struct EncodingAndInst {
   const Record *EncodingDef;
   const CodeGenInstruction *Inst;
   StringRef HwModeName;
+  std::vector<struct BitValue> EncodingBits;
 
   EncodingAndInst(const Record *EncodingDef, const CodeGenInstruction *Inst,
                   StringRef HwModeName = "")
@@ -348,6 +349,26 @@ static const BitsInit &getBitsField(const Record &Def, StringRef FieldName) {
 // Representation of the instruction to work on.
 typedef std::vector<BitValue> insn_t;
 
+// Populates the insn given the uid.
+static insn_t getEncodingBits(const Record *EncodingDef, unsigned BitWidth) {
+  const BitsInit &Bits = getBitsField(*EncodingDef, "Inst");
+  insn_t Insn(std::max(BitWidth, Bits.getNumBits()), BitValue::BIT_UNSET);
+  // We may have a SoftFail bitmask, which specifies a mask where an encoding
+  // may differ from the value in "Inst" and yet still be valid, but the
+  // disassembler should return SoftFail instead of Success.
+  //
+  // This is used for marking UNPREDICTABLE instructions in the ARM world.
+  const RecordVal *RV = EncodingDef->getValue("SoftFail");
+  const BitsInit *SFBits = RV ? dyn_cast<BitsInit>(RV->getValue()) : nullptr;
+  for (unsigned i = 0; i < Bits.getNumBits(); ++i) {
+    if (SFBits && BitValue(*SFBits, i) == BitValue::BIT_TRUE)
+      Insn[i] = BitValue::BIT_UNSET;
+    else
+      Insn[i] = BitValue(Bits, i);
+  }
+  return Insn;
+}
+
 /// Extracts a NumBits long field from Insn, starting from StartBit.
 /// Returns the value of the field if all bits are well-known,
 /// otherwise std::nullopt.
@@ -554,28 +575,11 @@ class FilterChooser {
 
   unsigned getBitWidth() const { return BitWidth; }
 
-protected:
-  // Populates the insn given the uid.
-  insn_t insnWithID(unsigned Opcode) const {
-    const Record *EncodingDef = AllInstructions[Opcode].EncodingDef;
-    const BitsInit &Bits = getBitsField(*EncodingDef, "Inst");
-    insn_t Insn(std::max(BitWidth, Bits.getNumBits()), BitValue::BIT_UNSET);
-    // We may have a SoftFail bitmask, which specifies a mask where an encoding
-    // may differ from the value in "Inst" and yet still be valid, but the
-    // disassembler should return SoftFail instead of Success.
-    //
-    // This is used for marking UNPREDICTABLE instructions in the ARM world.
-    const RecordVal *RV = EncodingDef->getValue("SoftFail");
-    const BitsInit *SFBits = RV ? dyn_cast<BitsInit>(RV->getValue()) : nullptr;
-    for (unsigned i = 0; i < Bits.getNumBits(); ++i) {
-      if (SFBits && BitValue(*SFBits, i) == BitValue::BIT_TRUE)
-        Insn[i] = BitValue::BIT_UNSET;
-      else
-        Insn[i] = BitValue(Bits, i);
-    }
-    return Insn;
+  ArrayRef<BitValue> getEncodingBits(unsigned EncodingID) const {
+    return AllInstructions[EncodingID].EncodingBits;
   }
 
+protected:
   /// dumpFilterArray - dumpFilterArray prints out debugging info for the given
   /// filter array as a series of chars.
   void dumpFilterArray(raw_ostream &OS, ArrayRef<BitValue> Filter) const;
@@ -669,7 +673,7 @@ Filter::Filter(const FilterChooser &owner, unsigned startBit, unsigned numBits)
 
   for (const auto &OpcPair : Owner.Opcodes) {
     // Populates the insn given the uid.
-    insn_t Insn = Owner.insnWithID(OpcPair.EncodingID);
+    ArrayRef<BitValue> Insn = Owner.getEncodingBits(OpcPair.EncodingID);
 
     // Scans the segment for possibly well-specified encoding bits.
     std::optional<uint64_t> Field = fieldFromInsn(Insn, StartBit, NumBits);
@@ -1420,7 +1424,7 @@ void FilterChooser::emitSoftFailTableEntry(DecoderTableInfo &TableInfo,
 // Emits table entries to decode the singleton.
 void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo,
                                             EncodingIDAndOpcode Opc) const {
-  insn_t Insn = insnWithID(Opc.EncodingID);
+  ArrayRef<BitValue> Insn = getEncodingBits(Opc.EncodingID);
 
   // Look for islands of undecoded bits of the singleton.
   std::vector<Island> Islands = getIslands(Insn);
@@ -1524,7 +1528,7 @@ bool FilterChooser::filterProcessor(ArrayRef<bitAttr_t> BitAttrs,
     assert(Opcodes.size() == 3);
 
     for (const auto &Opcode : Opcodes) {
-      insn_t Insn = insnWithID(Opcode.EncodingID);
+      ArrayRef<BitValue> Insn = getEncodingBits(Opcode.EncodingID);
 
       // Look for islands of undecoded bits of any instruction.
       std::vector<Island> Islands = getIslands(Insn);
@@ -1713,7 +1717,7 @@ void FilterChooser::doFilter() {
       BitAttrs[BitIndex] = ATTR_FILTERED;
 
   for (const EncodingIDAndOpcode &OpcPair : Opcodes) {
-    insn_t EncodingBits = insnWithID(OpcPair.EncodingID);
+    ArrayRef<BitValue> EncodingBits = getEncodingBits(OpcPair.EncodingID);
 
     for (unsigned BitIndex = 0; BitIndex < BitWidth; ++BitIndex) {
       switch (BitAttrs[BitIndex]) {
@@ -2579,10 +2583,17 @@ namespace {
   unsigned OpcodeMask = 0;
   for (const auto &[NSAndByteSize, EncodingIDs] : OpcMap) {
     const std::string &DecoderNamespace = NSAndByteSize.first;
-    const unsigned BitWidth = 8 * NSAndByteSize.second;
+    unsigned BitWidth = IsVarLenInst ? MaxInstLen : 8 * NSAndByteSize.second;
+
+    // Populate encoding bits in this group of encodings.
+    for (EncodingIDAndOpcode OpcPair : EncodingIDs) {
+      EncodingAndInst &Enc = NumberedEncodings[OpcPair.EncodingID];
+      assert(Enc.EncodingBits.empty() && "Already populated?");
+      Enc.EncodingBits = getEncodingBits(Enc.EncodingDef, BitWidth);
+    }
+
     // Emit the decoder for this namespace+width combination.
-    FilterChooser FC(NumberedEncodings, EncodingIDs, Operands,
-                     IsVarLenInst ? MaxInstLen : BitWidth, this);
+    FilterChooser FC(NumberedEncodings, EncodingIDs, Operands, BitWidth, this);
 
     // The decode table is cleared for each top level decoder function. The
     // predicates and decoders themselves, however, are shared across all

@s-barannikov s-barannikov force-pushed the tablegen/decoder/populate-encoding-once branch from 8ce1ca3 to 988a3dd Compare August 17, 2025 20:18
@s-barannikov

This comment was marked as outdated.

@s-barannikov s-barannikov marked this pull request as draft August 18, 2025 09:21
@s-barannikov s-barannikov force-pushed the tablegen/decoder/populate-encoding-once branch 3 times, most recently from 6b0124d to 1111145 Compare August 21, 2025 09:25
@jurahul jurahul removed their request for review August 21, 2025 17:41
@s-barannikov s-barannikov force-pushed the tablegen/decoder/populate-encoding-once branch 3 times, most recently from a8bce78 to 13b97a3 Compare August 22, 2025 00:13
`insnWithID` is called multiple times for the same encoding ID.
Call it once and store the result in `NumberedEncodings`.
Also, give the method a more meaningful name.
@s-barannikov s-barannikov force-pushed the tablegen/decoder/populate-encoding-once branch from 13b97a3 to 983d8be Compare August 22, 2025 00:15
@s-barannikov s-barannikov changed the title [TableGen][DecoderEmitter] Calculate encoding bits once (NFC) [TableGen][DecoderEmitter] Calculate encoding bits once Aug 22, 2025
@s-barannikov s-barannikov marked this pull request as ready for review August 22, 2025 00:19
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@s-barannikov s-barannikov merged commit 418fb50 into llvm:main Aug 22, 2025
11 checks passed
@s-barannikov s-barannikov deleted the tablegen/decoder/populate-encoding-once branch August 22, 2025 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants