[TableGen] Emit llvm::is_contained for CheckOpcode predicate

[wangpc-pp]

When the list is large, using llvm::is_contained is of higher
performance than a sequence of comparisons. When the list is small,
the llvm::is_contained can be inlined and unrolled, which has the
same effect as using a sequence of comparisons.

And the generated code is more readable.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Pengcheng Wang (wangpc-pp)

Changes

When the list is large, using llvm::is_contained is of higher
performance than a sequence of comparisons. And the generated code
is more readable.

---

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

2 Files Affected:

• (modified) llvm/test/TableGen/MacroFusion.td (+54)
• (modified) llvm/utils/TableGen/Common/PredicateExpander.cpp (+12)

diff --git a/llvm/test/TableGen/MacroFusion.td b/llvm/test/TableGen/MacroFusion.td
index 6cf22f5447150..c0b41ae31c082 100644
--- a/llvm/test/TableGen/MacroFusion.td
+++ b/llvm/test/TableGen/MacroFusion.td
@@ -35,6 +35,11 @@ def Inst0 : TestInst<0>;
 def Inst1 : TestInst<1>;
 let isCommutable = true in
 def Inst2 : TestInst<2>;
+def Inst3 : TestInst<3>;
+def Inst4 : TestInst<4>;
+def Inst5 : TestInst<5>;
+def Inst6 : TestInst<6>;
+def Inst7 : TestInst<7>;
 
 def BothFusionPredicate: BothFusionPredicateWithMCInstPredicate<CheckRegOperand<0, X0>>;
 def TestBothFusionPredicate: Fusion<"test-both-fusion-predicate", "HasBothFusionPredicate",
@@ -62,6 +67,11 @@ def TestSingleFusion: SingleFusion<"test-single-fusion", "HasTestSingleFusion",
                                    Inst0, Inst2,
                                    secondInstPred=CheckRegOperand<0, X0>>;
 
+def TestLargeCheckOpcodeFusion: SimpleFusion<"test-large-check-opcode-fusion", "TestLargeCheckOpcodeFusion",
+                                             "Test Large CheckOpcode Fusion",
+                                             CheckOpcode<[Inst0, Inst1, Inst2, Inst3, Inst4, Inst5, Inst6]>,
+                                             CheckOpcode<[Inst0, Inst1, Inst2, Inst3, Inst4, Inst5, Inst6, Inst7]>>;
+
 // CHECK-PREDICATOR:       #ifdef GET_Test_MACRO_FUSION_PRED_DECL
 // CHECK-PREDICATOR-NEXT:  #undef GET_Test_MACRO_FUSION_PRED_DECL
 // CHECK-PREDICATOR-EMPTY:
@@ -69,6 +79,7 @@ def TestSingleFusion: SingleFusion<"test-single-fusion", "HasTestSingleFusion",
 // CHECK-PREDICATOR-NEXT:  bool isTestBothFusionPredicate(const TargetInstrInfo &, const TargetSubtargetInfo &, const MachineInstr *, const MachineInstr &);
 // CHECK-PREDICATOR-NEXT:  bool isTestCommutableFusion(const TargetInstrInfo &, const TargetSubtargetInfo &, const MachineInstr *, const MachineInstr &);   
 // CHECK-PREDICATOR-NEXT:  bool isTestFusion(const TargetInstrInfo &, const TargetSubtargetInfo &, const MachineInstr *, const MachineInstr &);
+// CHECK-PREDICATOR-NEXT:  bool isTestLargeCheckOpcodeFusion(const TargetInstrInfo &, const TargetSubtargetInfo &, const MachineInstr *, const MachineInstr &);
 // CHECK-PREDICATOR-NEXT:  bool isTestSingleFusion(const TargetInstrInfo &, const TargetSubtargetInfo &, const MachineInstr *, const MachineInstr &);
 // CHECK-PREDICATOR-NEXT:  } // end namespace llvm
 // CHECK-PREDICATOR-EMPTY:
@@ -180,6 +191,47 @@ def TestSingleFusion: SingleFusion<"test-single-fusion", "HasTestSingleFusion",
 // CHECK-PREDICATOR-NEXT:      return false;
 // CHECK-PREDICATOR-NEXT:    return true;
 // CHECK-PREDICATOR-NEXT:  }
+// CHECK-PREDICATOR-NEXT:  bool isTestLargeCheckOpcodeFusion(
+// CHECK-PREDICATOR-NEXT:      const TargetInstrInfo &TII,
+// CHECK-PREDICATOR-NEXT:      const TargetSubtargetInfo &STI,
+// CHECK-PREDICATOR-NEXT:      const MachineInstr *FirstMI,
+// CHECK-PREDICATOR-NEXT:      const MachineInstr &SecondMI) {
+// CHECK-PREDICATOR-NEXT:    {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} auto &MRI = SecondMI.getMF()->getRegInfo();
+// CHECK-PREDICATOR-NEXT:    {
+// CHECK-PREDICATOR-NEXT:      const MachineInstr *MI = &SecondMI;
+// CHECK-PREDICATOR-NEXT:      if (!llvm::is_contained({Test::Inst0, Test::Inst1, Test::Inst2, Test::Inst3, Test::Inst4, Test::Inst5, Test::Inst6, Test::Inst7}, MI->getOpcode()))
+// CHECK-PREDICATOR-NEXT:        return false;
+// CHECK-PREDICATOR-NEXT:    }
+// CHECK-PREDICATOR-NEXT:    if (!FirstMI)
+// CHECK-PREDICATOR-NEXT:      return true;
+// CHECK-PREDICATOR-NEXT:    {
+// CHECK-PREDICATOR-NEXT:      const MachineInstr *MI = FirstMI;
+// CHECK-PREDICATOR-NEXT:      if ((
+// CHECK-PREDICATOR-NEXT:          MI->getOpcode() != Test::Inst0
+// CHECK-PREDICATOR-NEXT:          && MI->getOpcode() != Test::Inst1
+// CHECK-PREDICATOR-NEXT:          && MI->getOpcode() != Test::Inst2
+// CHECK-PREDICATOR-NEXT:          && MI->getOpcode() != Test::Inst3
+// CHECK-PREDICATOR-NEXT:          && MI->getOpcode() != Test::Inst4
+// CHECK-PREDICATOR-NEXT:          && MI->getOpcode() != Test::Inst5
+// CHECK-PREDICATOR-NEXT:          && MI->getOpcode() != Test::Inst6
+// CHECK-PREDICATOR-NEXT:        ))
+// CHECK-PREDICATOR-NEXT:        return false;
+// CHECK-PREDICATOR-NEXT:    }
+// CHECK-PREDICATOR-NEXT:      if (!SecondMI.getOperand(0).getReg().isVirtual()) {
+// CHECK-PREDICATOR-NEXT:      if (SecondMI.getOperand(0).getReg() != SecondMI.getOperand(1).getReg())
+// CHECK-PREDICATOR-NEXT:        return false;
+// CHECK-PREDICATOR-NEXT:    }
+// CHECK-PREDICATOR-NEXT:    {
+// CHECK-PREDICATOR-NEXT:      Register FirstDest = FirstMI->getOperand(0).getReg();
+// CHECK-PREDICATOR-NEXT:      if (FirstDest.isVirtual() && !MRI.hasOneNonDBGUse(FirstDest))
+// CHECK-PREDICATOR-NEXT:        return false;
+// CHECK-PREDICATOR-NEXT:    }
+// CHECK-PREDICATOR-NEXT:    if (!(FirstMI->getOperand(0).isReg() &&
+// CHECK-PREDICATOR-NEXT:          SecondMI.getOperand(1).isReg() &&
+// CHECK-PREDICATOR-NEXT:          FirstMI->getOperand(0).getReg() == SecondMI.getOperand(1).getReg()))
+// CHECK-PREDICATOR-NEXT:      return false;
+// CHECK-PREDICATOR-NEXT:    return true;
+// CHECK-PREDICATOR-NEXT:  }
 // CHECK-PREDICATOR-NEXT:  bool isTestSingleFusion(
 // CHECK-PREDICATOR-NEXT:      const TargetInstrInfo &TII,
 // CHECK-PREDICATOR-NEXT:      const TargetSubtargetInfo &STI,
@@ -239,6 +291,7 @@ def TestSingleFusion: SingleFusion<"test-single-fusion", "HasTestSingleFusion",
 // CHECK-SUBTARGET: { "test-both-fusion-predicate", "Test BothFusionPredicate", Test::TestBothFusionPredicate
 // CHECK-SUBTARGET: { "test-commutable-fusion", "Test Commutable Fusion", Test::TestCommutableFusion
 // CHECK-SUBTARGET: { "test-fusion", "Test Fusion", Test::TestFusion
+// CHECK-SUBTARGET: { "test-large-check-opcode-fusion", "Test Large CheckOpcode Fusion", Test::TestLargeCheckOpcodeFusion
 // CHECK-SUBTARGET: { "test-single-fusion", "Test SingleFusion", Test::TestSingleFusion
 
 // Check that we have generated `getMacroFusions()` function.
@@ -249,6 +302,7 @@ def TestSingleFusion: SingleFusion<"test-single-fusion", "HasTestSingleFusion",
 // CHECK-SUBTARGET-NEXT:   if (hasFeature(Test::TestBothFusionPredicate)) Fusions.push_back(llvm::isTestBothFusionPredicate); 
 // CHECK-SUBTARGET-NEXT:   if (hasFeature(Test::TestCommutableFusion)) Fusions.push_back(llvm::isTestCommutableFusion); 
 // CHECK-SUBTARGET-NEXT:   if (hasFeature(Test::TestFusion)) Fusions.push_back(llvm::isTestFusion);
+// CHECK-SUBTARGET-NEXT:   if (hasFeature(Test::TestLargeCheckOpcodeFusion)) Fusions.push_back(llvm::isTestLargeCheckOpcodeFusion);
 // CHECK-SUBTARGET-NEXT:   if (hasFeature(Test::TestSingleFusion)) Fusions.push_back(llvm::isTestSingleFusion);
 // CHECK-SUBTARGET-NEXT:   return Fusions;
 // CHECK-SUBTARGET-NEXT: }
diff --git a/llvm/utils/TableGen/Common/PredicateExpander.cpp b/llvm/utils/TableGen/Common/PredicateExpander.cpp
index e54df89937c4a..37ff88c4af6e3 100644
--- a/llvm/utils/TableGen/Common/PredicateExpander.cpp
+++ b/llvm/utils/TableGen/Common/PredicateExpander.cpp
@@ -152,6 +152,18 @@ void PredicateExpander::expandCheckOpcode(raw_ostream &OS,
     return;
   }
 
+  if (Opcodes.size() >= 8) {
+    OS << (shouldNegate() ? "!" : "") << "llvm::is_contained(";
+    ListSeparator Sep;
+    OS << "{";
+    for (const Record *Inst : Opcodes)
+      OS << Sep << Inst->getValueAsString("Namespace")
+         << "::" << Inst->getName();
+    OS << "}";
+    OS << ", MI" << (isByRef() ? "." : "->") << "getOpcode())";
+    return;
+  }
+
   OS << '(';
   ++Indent;
   for (const Record *Rec : Opcodes) {

[arsenm]

Is this an experimentally determined number? Does it hurt to just always do this?

[wangpc-pp]

It is determined by the estimated instruction count when the list is large: https://godbolt.org/z/3qGeWfjM9.
Yeah, I think it worths being the default behavior since is_contained can be inlined and unrolled for small sizes.