Skip to content

Conversation

s-barannikov
Copy link
Contributor

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

Extract findBestFilter() const searching for the best filter and move calls to recurse() out of it to a single place.

Extract dump() as well, it is useful for debugging.

@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2025

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

Extract findBestFilter() const searching for the best filter and move calls to non-const recurse() out of it to a single place.

Extract dump() as well, it is useful for debugging.


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

1 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+55-51)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index e1c38192fe18f..892c3a246d6a1 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -612,19 +612,19 @@ class FilterChooser {
   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);
-
   // reportRegion is a helper function for filterProcessor to mark a region as
   // eligible for use as a filter region.
   void reportRegion(std::vector<std::unique_ptr<Filter>> &Filters, bitAttr_t RA,
-                    unsigned StartBit, unsigned BitIndex, bool AllowMixed);
+                    unsigned StartBit, unsigned BitIndex,
+                    bool AllowMixed) const;
+
+  /// Scans the well-known encoding bits of the encodings and, builds up a list
+  /// of candidate filters, and then returns the best one, if any.
+  std::unique_ptr<Filter> findBestFilter(ArrayRef<bitAttr_t> BitAttrs,
+                                         bool AllowMixed,
+                                         bool Greedy = true) const;
 
-  // FilterProcessor scans the well-known encoding bits of the instructions and
-  // builds up a list of candidate filters.  It chooses the best filter and
-  // recursively descends down the decoding tree.
-  bool filterProcessor(ArrayRef<bitAttr_t> BitAttrs, bool AllowMixed,
-                       bool Greedy = true);
+  std::unique_ptr<Filter> findBestFilter() const;
 
   // Decides on the best configuration of filter(s) to use in order to decode
   // the instructions.  A conflict of instructions may occur, in which case we
@@ -635,6 +635,8 @@ class FilterChooser {
   // emitTableEntries - Emit state machine entries to decode our share of
   // instructions.
   void emitTableEntries(DecoderTableInfo &TableInfo) const;
+
+  void dump() const;
 };
 
 } // end anonymous namespace
@@ -1456,28 +1458,19 @@ void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo,
   Best.getVariableFC().emitTableEntries(TableInfo);
 }
 
-// Assign a single filter and run with it.  Top level API client can initialize
-// with a single filter to start the filtering process.
-void FilterChooser::runSingleFilter(unsigned startBit, unsigned numBit) {
-  BestFilter = std::make_unique<Filter>(*this, startBit, numBit);
-  BestFilter->recurse();
-}
-
 // reportRegion is a helper function for filterProcessor to mark a region as
 // eligible for use as a filter region.
 void FilterChooser::reportRegion(std::vector<std::unique_ptr<Filter>> &Filters,
                                  bitAttr_t RA, unsigned StartBit,
-                                 unsigned BitIndex, bool AllowMixed) {
+                                 unsigned BitIndex, bool AllowMixed) const {
   if (AllowMixed ? RA == ATTR_MIXED : RA == ATTR_ALL_SET)
     Filters.push_back(
         std::make_unique<Filter>(*this, StartBit, BitIndex - StartBit));
 }
 
-// FilterProcessor scans the well-known encoding bits of the instructions and
-// builds up a list of candidate filters.  It chooses the best filter and
-// recursively descends down the decoding tree.
-bool FilterChooser::filterProcessor(ArrayRef<bitAttr_t> BitAttrs,
-                                    bool AllowMixed, bool Greedy) {
+std::unique_ptr<Filter>
+FilterChooser::findBestFilter(ArrayRef<bitAttr_t> BitAttrs, bool AllowMixed,
+                               bool Greedy) const {
   assert(EncodingIDs.size() >= 2 && "Nothing to filter");
 
   // Heuristics.  See also doFilter()'s "Heuristics" comment when num of
@@ -1493,8 +1486,8 @@ bool FilterChooser::filterProcessor(ArrayRef<bitAttr_t> BitAttrs,
       std::vector<Island> Islands = getIslands(EncodingBits);
       if (!Islands.empty()) {
         // Found an instruction with island(s).  Now just assign a filter.
-        runSingleFilter(Islands[0].StartBit, Islands[0].NumBits);
-        return true;
+        return std::make_unique<Filter>(*this, Islands[0].StartBit,
+                                        Islands[0].NumBits);
       }
     }
   }
@@ -1633,24 +1626,12 @@ bool FilterChooser::filterProcessor(ArrayRef<bitAttr_t> BitAttrs,
   }
 
   if (AllUseless)
-    return false;
-
-  BestFilter = std::move(Filters[BestIndex]);
-  BestFilter->recurse();
-  return true;
-
-} // end of FilterChooser::filterProcessor(bool)
-
-// Decides on the best configuration of filter(s) to use in order to decode
-// the instructions.  A conflict of instructions may occur, in which case we
-// dump the conflict set to the standard error.
-void FilterChooser::doFilter() {
-  assert(!EncodingIDs.empty() && "FilterChooser created with no instructions");
+    return nullptr;
 
-  // No filter needed.
-  if (EncodingIDs.size() < 2)
-    return;
+  return std::move(Filters[BestIndex]);
+}
 
+std::unique_ptr<Filter> FilterChooser::findBestFilter() const {
   // We maintain BIT_WIDTH copies of the bitAttrs automaton.
   // The automaton consumes the corresponding bit from each
   // instruction.
@@ -1706,32 +1687,56 @@ void FilterChooser::doFilter() {
   }
 
   // Try regions of consecutive known bit values first.
-  if (filterProcessor(BitAttrs, /*AllowMixed=*/false))
-    return;
+  if (std::unique_ptr<Filter> F =
+          findBestFilter(BitAttrs, /*AllowMixed=*/false))
+    return F;
 
   // Then regions of mixed bits (both known and unitialized bit values allowed).
-  if (filterProcessor(BitAttrs, /*AllowMixed=*/true))
-    return;
+  if (std::unique_ptr<Filter> F = findBestFilter(BitAttrs, /*AllowMixed=*/true))
+    return F;
 
   // Heuristics to cope with conflict set {t2CMPrs, t2SUBSrr, t2SUBSrs} where
   // no single instruction for the maximum ATTR_MIXED region Inst{14-4} has a
   // well-known encoding pattern.  In such case, we backtrack and scan for the
   // the very first consecutive ATTR_ALL_SET region and assign a filter to it.
-  if (EncodingIDs.size() == 3 &&
-      filterProcessor(BitAttrs, /*AllowMixed=*/true, /*Greedy=*/false))
+  if (EncodingIDs.size() == 3) {
+    if (std::unique_ptr<Filter> F =
+            findBestFilter(BitAttrs, /*AllowMixed=*/true, /*Greedy=*/false))
+      return F;
+  }
+
+  // There is a conflict we could not resolve.
+  return nullptr;
+}
+
+// Decides on the best configuration of filter(s) to use in order to decode
+// the instructions.  A conflict of instructions may occur, in which case we
+// dump the conflict set to the standard error.
+void FilterChooser::doFilter() {
+  assert(!EncodingIDs.empty() && "FilterChooser created with no instructions");
+
+  // No filter needed.
+  if (EncodingIDs.size() < 2)
     return;
 
-  // We don't know how to decode these instructions! Dump the
-  // conflict set and bail.
-  assert(!BestFilter);
+  BestFilter = findBestFilter();
+  if (BestFilter) {
+    BestFilter->recurse();
+    return;
+  }
 
   // Print out useful conflict information for postmortem analysis.
   errs() << "Decoding Conflict:\n";
+  dump();
+  PrintFatalError("Decoding conflict encountered");
+}
 
-  // Dump filters.
+void FilterChooser::dump() const {
   indent Indent(4);
   // Helps to keep the output right-justified.
   unsigned PadToWidth = getMaxEncodingWidth();
+
+  // Dump filter stack.
   dumpStack(errs(), Indent, PadToWidth);
 
   // Dump encodings.
@@ -1741,7 +1746,6 @@ void FilterChooser::doFilter() {
     printKnownBits(errs(), Encoding.getMandatoryBits(), '_');
     errs() << "  " << Encoding.getName() << '\n';
   }
-  PrintFatalError("Decoding conflict encountered");
 }
 
 // emitTableEntries - Emit state machine entries to decode our share of

@s-barannikov s-barannikov enabled auto-merge (squash) August 22, 2025 22:49
@github-actions
Copy link

github-actions bot commented Aug 22, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Extract `findBestFilter() const` searching for the best filter and
move calls to non-const `recurse()` out of it to a single place.

Extract `dump()` as well, it is useful for debugging.
@s-barannikov s-barannikov force-pushed the tablegen/decoder/find-best-filter branch from de623d5 to f63e644 Compare August 22, 2025 22:53
@s-barannikov s-barannikov disabled auto-merge August 22, 2025 22:54
@s-barannikov s-barannikov enabled auto-merge (squash) August 22, 2025 22:54
@s-barannikov s-barannikov merged commit 8aba413 into llvm:main Aug 22, 2025
9 checks passed
@s-barannikov s-barannikov deleted the tablegen/decoder/find-best-filter branch August 24, 2025 04:49
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.

2 participants