[TableGen][DecoderEmitter] Avoid using a sentinel value (#153986)

`NO_FIXED_SEGMENTS_SENTINEL` has a value that is actually a valid field
encoding and so it cannot be used as a sentinel.
Replace the sentinel with a new member variable, `VariableFC`, that
contains the value previously stored in `FilterChooserMap` with
`NO_FIXED_SEGMENTS_SENTINEL` key.

2 files changed, 76 insertions, 43 deletions

diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td b/llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td
new file mode 100644
index 0000000..b9da61d
--- /dev/null
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/big-filter.td
@@ -0,0 +1,40 @@
+// RUN: llvm-tblgen -gen-disassembler -I %p/../../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+class I : Instruction {
+  let InOperandList = (ins);
+  let OutOperandList = (outs);
+  let Size = 16;
+  bits<128> Inst;
+}
+
+// Check that a 64-bit filter with all bits set does not confuse DecoderEmitter.
+//
+// CHECK-LABEL: static const uint8_t DecoderTable128[] = {
+// CHECK-NEXT:    MCD::OPC_ExtractField, 0, 64,
+// CHECK-NEXT:    MCD::OPC_FilterValue, 1, 8, 0,
+// CHECK-NEXT:    MCD::OPC_CheckFieldOrFail, 127, 1, 1,
+// CHECK-NEXT:    MCD::OPC_Decode, 187, 2, 0,
+// CHECK-NEXT:    MCD::OPC_FilterValueOrFail, 255, 255, 255, 255, 255, 255, 255, 255, 255, 1,
+// CHECK-NEXT:    MCD::OPC_CheckFieldOrFail, 127, 1, 0,
+// CHECK-NEXT:    MCD::OPC_Decode, 186, 2, 0,
+// CHECK-NEXT:    MCD::OPC_Fail,
+// CHECK-NEXT:    0
+// CHECK-NEXT:  };
+
+def I1 : I {
+  let Inst{63...0} = -1;
+  let Inst{127} = 0;
+}
+
+def I2 : I {
+  let Inst{63...0} = 1;
+  let Inst{127} = 1;
+}
+
+def II : InstrInfo;
+
+def MyTarget : Target {
+  let InstructionSet = II;
+}
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index f69e47e..238c87a 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -360,9 +360,6 @@ fieldFromInsn(const insn_t &Insn, unsigned StartBit, unsigned NumBits) {
 
 namespace {
 
-static constexpr uint64_t NO_FIXED_SEGMENTS_SENTINEL =
-    std::numeric_limits<uint64_t>::max();
-
 class FilterChooser;
 
 /// Filter - Filter works with FilterChooser to produce the decoding tree for
@@ -416,6 +413,9 @@ protected:
   // Map of well-known segment value to its delegate.
   std::map<uint64_t, std::unique_ptr<const FilterChooser>> FilterChooserMap;
 
+  // A filter chooser for encodings that contain some '?' in the filtered range.
+  std::unique_ptr<const FilterChooser> VariableFC;
+
   // Number of instructions which fall under FilteredInstructions category.
   unsigned NumFiltered;
 
@@ -435,8 +435,8 @@ public:
   // Return the filter chooser for the group of instructions without constant
   // segment values.
   const FilterChooser &getVariableFC() const {
-    assert(NumFiltered == 1 && FilterChooserMap.size() == 1);
-    return *(FilterChooserMap.find(NO_FIXED_SEGMENTS_SENTINEL)->second);
+    assert(NumFiltered == 1 && FilterChooserMap.empty());
+    return *VariableFC;
   }
 
   // Divides the decoding task into sub tasks and delegates them to the
@@ -647,7 +647,7 @@ Filter::Filter(Filter &&f)
       FilteredIDs(std::move(f.FilteredIDs)),
       VariableIDs(std::move(f.VariableIDs)),
       FilterChooserMap(std::move(f.FilterChooserMap)),
-      NumFiltered(f.NumFiltered) {}
+      VariableFC(std::move(f.VariableFC)), NumFiltered(f.NumFiltered) {}
 
 Filter::Filter(const FilterChooser &owner, unsigned startBit, unsigned numBits)
     : Owner(owner), StartBit(startBit), NumBits(numBits) {
@@ -694,16 +694,15 @@ void Filter::recurse() {
 
     // Delegates to an inferior filter chooser for further processing on this
     // group of instructions whose segment values are variable.
-    FilterChooserMap.try_emplace(
-        NO_FIXED_SEGMENTS_SENTINEL,
+    VariableFC =
         std::make_unique<FilterChooser>(Owner.AllInstructions, VariableIDs,
-                                        Owner.Operands, BitValueArray, Owner));
+                                        Owner.Operands, BitValueArray, Owner);
   }
 
   // No need to recurse for a singleton filtered instruction.
   // See also Filter::emit*().
   if (getNumFiltered() == 1) {
-    assert(FilterChooserMap.size() == 1);
+    assert(VariableFC && "Shouldn't have created a filter for one encoding!");
     return;
   }
 
@@ -733,46 +732,30 @@ void Filter::emitTableEntry(DecoderTableInfo &TableInfo) const {
   TableInfo.Table.insertULEB128(StartBit);
   TableInfo.Table.push_back(NumBits);
 
-  // If the NO_FIXED_SEGMENTS_SENTINEL is present, we need to add a new scope
-  // for this filter. Otherwise, we can skip adding a new scope and any
-  // patching added will automatically be added to the enclosing scope.
-
-  // If NO_FIXED_SEGMENTS_SENTINEL is present, it will be last entry in
-  // FilterChooserMap.
-
+  // If VariableFC is present, we need to add a new scope for this filter.
+  // Otherwise, we can skip adding a new scope and any patching added will
+  // automatically be added to the enclosing scope.
   const uint64_t LastFilter = FilterChooserMap.rbegin()->first;
-  bool HasFallthrough = LastFilter == NO_FIXED_SEGMENTS_SENTINEL;
-  if (HasFallthrough)
-    TableInfo.pushScope();
+  if (VariableFC)
+    TableInfo.FixupStack.emplace_back();
 
   DecoderTable &Table = TableInfo.Table;
 
   size_t PrevFilter = 0;
   for (const auto &[FilterVal, Delegate] : FilterChooserMap) {
-    // Field value NO_FIXED_SEGMENTS_SENTINEL implies a non-empty set of
-    // variable instructions. See also recurse().
-    if (FilterVal == NO_FIXED_SEGMENTS_SENTINEL) {
-      // Each scope should always have at least one filter value to check
-      // for.
-      assert(PrevFilter != 0 && "empty filter set!");
-      TableInfo.popScope();
-      PrevFilter = 0; // Don't re-process the filter's fallthrough.
+    // The last filtervalue emitted can be OPC_FilterValue if we are at
+    // outermost scope.
+    const uint8_t DecoderOp =
+        FilterVal == LastFilter && TableInfo.isOutermostScope()
+            ? MCD::OPC_FilterValueOrFail
+            : MCD::OPC_FilterValue;
+    Table.push_back(DecoderOp);
+    Table.insertULEB128(FilterVal);
+    if (DecoderOp == MCD::OPC_FilterValue) {
+      // Reserve space for the NumToSkip entry. We'll backpatch the value later.
+      PrevFilter = Table.insertNumToSkip();
     } else {
-      // The last filtervalue emitted can be OPC_FilterValue if we are at
-      // outermost scope.
-      const uint8_t DecoderOp =
-          FilterVal == LastFilter && TableInfo.isOutermostScope()
-              ? MCD::OPC_FilterValueOrFail
-              : MCD::OPC_FilterValue;
-      Table.push_back(DecoderOp);
-      Table.insertULEB128(FilterVal);
-      if (DecoderOp == MCD::OPC_FilterValue) {
-        // Reserve space for the NumToSkip entry. We'll backpatch the value
-        // later.
-        PrevFilter = Table.insertNumToSkip();
-      } else {
-        PrevFilter = 0;
-      }
+      PrevFilter = 0;
     }
 
     // We arrive at a category of instructions with the same segment value.
@@ -787,6 +770,16 @@ void Filter::emitTableEntry(DecoderTableInfo &TableInfo) const {
       Table.patchNumToSkip(PrevFilter, Table.size());
   }
 
+  if (VariableFC) {
+    // Each scope should always have at least one filter value to check for.
+    assert(PrevFilter != 0 && "empty filter set!");
+    TableInfo.popScope();
+    PrevFilter = 0; // Don't re-process the filter's fallthrough.
+
+    // Delegate to the sub filter chooser for further decoding.
+    VariableFC->emitTableEntries(TableInfo);
+  }
+
   // If there is no fallthrough and the final filter was not in the outermost
   // scope, then it must be fixed up according to the enclosing scope rather
   // than the current position.