[llvm] [NFC][DecoderEmitter] Predicate generation code cleanup (PR #158140)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 14 17:15:14 PDT 2025


https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/158140

>From 6bbae7ddec0f8855e6714af9b93b6c351c7f3935 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Thu, 11 Sep 2025 12:33:42 -0700
Subject: [PATCH 1/3] [NFC][DecoderEmitter] Predicate generation code cleanup

Use `ListSeparator` to join individual predicated with `&&`.
Eliminate `doesOpcodeNeedPredicate` and instead have
`emitPredicateMatch` return true if any predicates were generated.
---
 llvm/utils/TableGen/DecoderEmitter.cpp | 32 ++++++--------------------
 1 file changed, 7 insertions(+), 25 deletions(-)

diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index a8a9036a1a7f4..9dcf1528503e2 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -634,8 +634,6 @@ class DecoderTableBuilder {
 
   bool emitPredicateMatch(raw_ostream &OS, unsigned EncodingID) const;
 
-  bool doesOpcodeNeedPredicate(unsigned EncodingID) const;
-
   void emitPredicateTableEntry(unsigned EncodingID) const;
 
   void emitSoftFailTableEntry(unsigned EncodingID) const;
@@ -1218,7 +1216,8 @@ bool DecoderTableBuilder::emitPredicateMatch(raw_ostream &OS,
                                              unsigned EncodingID) const {
   const ListInit *Predicates =
       Encodings[EncodingID].getRecord()->getValueAsListInit("Predicates");
-  bool IsFirstEmission = true;
+  ListSeparator LS(" && ");
+  bool AnyPredicate = false;
   for (unsigned i = 0; i < Predicates->size(); ++i) {
     const Record *Pred = Predicates->getElementAsRecord(i);
     if (!Pred->getValue("AssemblerMatcherPredicate"))
@@ -1227,28 +1226,13 @@ bool DecoderTableBuilder::emitPredicateMatch(raw_ostream &OS,
     if (!isa<DagInit>(Pred->getValue("AssemblerCondDag")->getValue()))
       continue;
 
-    if (!IsFirstEmission)
-      OS << " && ";
+    AnyPredicate = true;
+    OS << LS;
     if (emitPredicateMatchAux(*Pred->getValueAsDag("AssemblerCondDag"),
                               Predicates->size() > 1, OS))
       PrintFatalError(Pred->getLoc(), "Invalid AssemblerCondDag!");
-    IsFirstEmission = false;
-  }
-  return !Predicates->empty();
-}
-
-bool DecoderTableBuilder::doesOpcodeNeedPredicate(unsigned EncodingID) const {
-  const ListInit *Predicates =
-      Encodings[EncodingID].getRecord()->getValueAsListInit("Predicates");
-  for (unsigned i = 0; i < Predicates->size(); ++i) {
-    const Record *Pred = Predicates->getElementAsRecord(i);
-    if (!Pred->getValue("AssemblerMatcherPredicate"))
-      continue;
-
-    if (isa<DagInit>(Pred->getValue("AssemblerCondDag")->getValue()))
-      return true;
   }
-  return false;
+  return AnyPredicate;
 }
 
 unsigned DecoderTableBuilder::getPredicateIndex(StringRef Predicate) const {
@@ -1266,15 +1250,13 @@ unsigned DecoderTableBuilder::getPredicateIndex(StringRef Predicate) const {
 }
 
 void DecoderTableBuilder::emitPredicateTableEntry(unsigned EncodingID) const {
-  if (!doesOpcodeNeedPredicate(EncodingID))
-    return;
-
   // Build up the predicate string.
   SmallString<256> Predicate;
   // FIXME: emitPredicateMatch() functions can take a buffer directly rather
   // than a stream.
   raw_svector_ostream PS(Predicate);
-  emitPredicateMatch(PS, EncodingID);
+  if (!emitPredicateMatch(PS, EncodingID))
+    return;
 
   // Figure out the index into the predicate table for the predicate just
   // computed.

>From b04eb3bff602752ed298a4fcf09da4f39bb7c730 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Sun, 14 Sep 2025 07:01:35 -0700
Subject: [PATCH 2/3] Review feedback

---
 .../TableGen/Common/SubtargetFeatureInfo.cpp  | 15 +++++---
 llvm/utils/TableGen/DecoderEmitter.cpp        | 38 +++++++------------
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp b/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp
index bd8af32ee5096..5239287853322 100644
--- a/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp
+++ b/llvm/utils/TableGen/Common/SubtargetFeatureInfo.cpp
@@ -190,8 +190,9 @@ void SubtargetFeatureInfo::emitMCPredicateCheck(
   bool ParenIfBinOp = range_size(MCPredicates) > 1;
   for (const Record *R : MCPredicates) {
     OS << LS;
-    emitFeaturesAux(TargetName, *R->getValueAsDag("AssemblerCondDag"),
-                    ParenIfBinOp, OS);
+    if (emitFeaturesAux(TargetName, *R->getValueAsDag("AssemblerCondDag"),
+                    ParenIfBinOp, OS))
+      PrintFatalError(R, "Invalid AssemblerCondDag!");
   }
 }
 
@@ -206,12 +207,14 @@ void SubtargetFeatureInfo::emitComputeAssemblerAvailableFeatures(
     OS << "const ";
   OS << "{\n";
   OS << "  FeatureBitset Features;\n";
-  for (const auto &SF : SubtargetFeatures) {
-    const SubtargetFeatureInfo &SFI = SF.second;
+  for (const SubtargetFeatureInfo &SFI : make_second_range(SubtargetFeatures)) {
+    const Record *Def = SFI.TheDef;
 
     OS << "  if (";
-    emitFeaturesAux(TargetName, *SFI.TheDef->getValueAsDag("AssemblerCondDag"),
-                    /*ParenIfBinOp=*/false, OS);
+    if (emitFeaturesAux(TargetName, *Def->getValueAsDag("AssemblerCondDag"),
+                    /*ParenIfBinOp=*/false, OS))
+      PrintFatalError(Def, "Invalid AssemblerCondDag!");
+
     OS << ")\n";
     OS << "    Features.set(" << SFI.getEnumBitName() << ");\n";
   }
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 9dcf1528503e2..f2d8b2d9a5c91 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -16,6 +16,7 @@
 #include "Common/CodeGenTarget.h"
 #include "Common/InfoByHwMode.h"
 #include "Common/VarLenCodeEmitterGen.h"
+#include "Common/SubtargetFeatureInfo.h"
 #include "TableGenBackends.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -632,7 +633,7 @@ class DecoderTableBuilder {
   bool emitPredicateMatchAux(const Init &Val, bool ParenIfBinOp,
                              raw_ostream &OS) const;
 
-  bool emitPredicateMatch(raw_ostream &OS, unsigned EncodingID) const;
+  void emitPredicateMatch(raw_ostream &OS, unsigned EncodingID) const;
 
   void emitPredicateTableEntry(unsigned EncodingID) const;
 
@@ -926,7 +927,7 @@ void DecoderEmitter::emitPredicateFunction(formatted_raw_ostream &OS,
   // The predicate function is just a big switch statement based on the
   // input predicate index.
   OS << "static bool checkDecoderPredicate(unsigned Idx, const FeatureBitset "
-        "&Bits) {\n";
+        "&FB) {\n";
   OS << "  switch (Idx) {\n";
   OS << "  default: llvm_unreachable(\"Invalid index!\");\n";
   for (const auto &[Index, Predicate] : enumerate(Predicates)) {
@@ -1212,27 +1213,14 @@ bool DecoderTableBuilder::emitPredicateMatchAux(const Init &Val,
   return true;
 }
 
-bool DecoderTableBuilder::emitPredicateMatch(raw_ostream &OS,
+void DecoderTableBuilder::emitPredicateMatch(raw_ostream &OS,
                                              unsigned EncodingID) const {
-  const ListInit *Predicates =
+  const ListInit *PredicateList =
       Encodings[EncodingID].getRecord()->getValueAsListInit("Predicates");
-  ListSeparator LS(" && ");
-  bool AnyPredicate = false;
-  for (unsigned i = 0; i < Predicates->size(); ++i) {
-    const Record *Pred = Predicates->getElementAsRecord(i);
-    if (!Pred->getValue("AssemblerMatcherPredicate"))
-      continue;
-
-    if (!isa<DagInit>(Pred->getValue("AssemblerCondDag")->getValue()))
-      continue;
-
-    AnyPredicate = true;
-    OS << LS;
-    if (emitPredicateMatchAux(*Pred->getValueAsDag("AssemblerCondDag"),
-                              Predicates->size() > 1, OS))
-      PrintFatalError(Pred->getLoc(), "Invalid AssemblerCondDag!");
-  }
-  return AnyPredicate;
+  std::vector<const Record *> Predicates;
+  for (unsigned i = 0; i < PredicateList->size(); ++i)
+    Predicates.push_back(PredicateList->getElementAsRecord(i));
+  SubtargetFeatureInfo::emitMCPredicateCheck(OS, Target.getName(), Predicates);
 }
 
 unsigned DecoderTableBuilder::getPredicateIndex(StringRef Predicate) const {
@@ -1252,10 +1240,12 @@ unsigned DecoderTableBuilder::getPredicateIndex(StringRef Predicate) const {
 void DecoderTableBuilder::emitPredicateTableEntry(unsigned EncodingID) const {
   // Build up the predicate string.
   SmallString<256> Predicate;
-  // FIXME: emitPredicateMatch() functions can take a buffer directly rather
-  // than a stream.
   raw_svector_ostream PS(Predicate);
-  if (!emitPredicateMatch(PS, EncodingID))
+  emitPredicateMatch(PS, EncodingID);
+  // Predicate being empty indicates that there are no predicates.
+  // Predicate being "false" indicate that there are predicates but with no
+  // AssemblerMatcherPredicate.
+  if (Predicate.empty() || Predicate == "false")
     return;
 
   // Figure out the index into the predicate table for the predicate just

>From c1f1dec31b55b4abb5eb47c5baa066200484b72d Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Sun, 14 Sep 2025 17:14:40 -0700
Subject: [PATCH 3/3] Drop outer ()

---
 llvm/test/TableGen/AsmPredicateCombining.td     | 10 +++++-----
 llvm/test/TableGen/AsmPredicateCondsEmission.td |  2 +-
 llvm/utils/TableGen/DecoderEmitter.cpp          |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/llvm/test/TableGen/AsmPredicateCombining.td b/llvm/test/TableGen/AsmPredicateCombining.td
index c8081a428d7bb..c86bb9dc24330 100644
--- a/llvm/test/TableGen/AsmPredicateCombining.td
+++ b/llvm/test/TableGen/AsmPredicateCombining.td
@@ -63,19 +63,19 @@ def AsmPred4 : Predicate<"Pred4">, AssemblerPredicate<(all_of AsmCond4, (not (an
 // MATCHER-NEXT:   Features.set(Feature_AsmPred4Bit);
 
 def insn1 : TestInsn<1, [AsmPred1]>;
-// DISASS: return (Bits[arch::AsmCond1]);
+// DISASS: return FB[arch::AsmCond1]
 
 def insn2 : TestInsn<2, [AsmPred2]>;
-// DISASS: return (Bits[arch::AsmCond2a] && Bits[arch::AsmCond2b])
+// DISASS: return FB[arch::AsmCond2a] && FB[arch::AsmCond2b]
 
 def insn3 : TestInsn<3, [AsmPred3]>;
-// DISASS: return (Bits[arch::AsmCond3a] || Bits[arch::AsmCond3b])
+// DISASS: return FB[arch::AsmCond3a] || FB[arch::AsmCond3b]
 
 def insn4 : TestInsn<4, [AsmPred1, AsmPred2]>;
-// DISASS: return (Bits[arch::AsmCond1] && (Bits[arch::AsmCond2a] && Bits[arch::AsmCond2b]))
+// DISASS: return FB[arch::AsmCond1] && (FB[arch::AsmCond2a] && FB[arch::AsmCond2b])
 
 def insn5 : TestInsn<5, [AsmPred1, AsmPred3]>;
-// DISASS: return (Bits[arch::AsmCond1] && (Bits[arch::AsmCond3a] || Bits[arch::AsmCond3b]))
+// DISASS: return FB[arch::AsmCond1] && (FB[arch::AsmCond3a] || FB[arch::AsmCond3b])
 
 def insn6 : TestInsn<6, []>;
 def : InstAlias<"alias1", (insn6 R0)> { let Predicates = [AsmPred1]; }
diff --git a/llvm/test/TableGen/AsmPredicateCondsEmission.td b/llvm/test/TableGen/AsmPredicateCondsEmission.td
index 7b2ab2afa5a8d..25bc1ab95a14f 100644
--- a/llvm/test/TableGen/AsmPredicateCondsEmission.td
+++ b/llvm/test/TableGen/AsmPredicateCondsEmission.td
@@ -29,4 +29,4 @@ def foo : Instruction {
   let Predicates = [Pred1, Pred2];
 }
 
-// CHECK: return (Bits[arch::AssemblerCondition2]);
+// CHECK: return FB[arch::AssemblerCondition2];
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index f2d8b2d9a5c91..55eb4cfab67b5 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -932,7 +932,7 @@ void DecoderEmitter::emitPredicateFunction(formatted_raw_ostream &OS,
   OS << "  default: llvm_unreachable(\"Invalid index!\");\n";
   for (const auto &[Index, Predicate] : enumerate(Predicates)) {
     OS << "  case " << Index << ":\n";
-    OS << "    return (" << Predicate << ");\n";
+    OS << "    return " << Predicate << ";\n";
   }
   OS << "  }\n";
   OS << "}\n\n";



More information about the llvm-commits mailing list