[GlobalISel] Optimized MatchData Handling

llvm/include/llvm/CodeGen/GlobalISel/Combiner.h

@@ -60,6 +60,56 @@ class Combiner : public GIMatchTableExecutor {
 
   bool combineMachineInstrs();
 
+  /// Base class for all dynamic MatchDatas definitions, used for type erasure
+  /// purposes.
+  ///
+  /// As derived classes may have non-trivial destructors, we need to be able to
+  /// call the destructor through the unique_ptr of the base class.
+  ///
+  /// There are two ways to achieve this.
+  ///   - (Easiest) Make MatchDataBase have a virtual destructor.
+  ///   - Use a custom deleter
+  ///
+  /// We use solution number 2, that way we don't have a vtable in all MatchData
+  /// objects (making them more compact), and we can have trivially destructible
+  /// MatchDatas, which avoids useless virtual function calls and allows the
+  /// compiler/libraries to use faster code (as it knows no destructor needs to
+  /// be called).
+  ///
+  /// This is really a micro-optimization, but MatchData used to be a
+  /// performance issue so better safe than sorry.
+  struct MatchDataBase {};
+
+  /// If a MatchData instance is active, cast it to Ty and return it.
+  /// Otherwise, create a new MatchData instance of type Ty and return it.
+  template <typename Ty> Ty &getOrCreateMatchData() const {
+    static_assert(std::is_base_of_v<MatchDataBase, Ty>,
+                  "Type must derive from MatchDataBase to be allocatable!");
+    // Allocate if we have no MatchData active, or if the MatchData that's
+    // active is not the one that we want.
+    if (!MatchData || Ty::ID != MatchDataID) {
+      MatchDataID = Ty::ID;
+      MatchData = std::unique_ptr<Ty, MatchDataDeleter>(
+          new Ty(), [](MatchDataBase *MD) { delete static_cast<Ty *>(MD); });
+    }
+    return *static_cast<Ty *>(MatchData.get());
+  }
+
+  /// Deallocates the currently active MatchData instance.
+  ///
+  /// TODO: Can we get away with never actually calling this? The MatchData
+  /// instance would then just be deleted by getOrCreateMatchData or when the
+  /// Combiner class is deallocated. I'm just a bit scared it'd cause issues
+  /// with captured values in some of the lambdas used as MatchInfo.
+  void resetMatchData() const { MatchData.reset(); }
+
+private:
+  using MatchDataDeleter = void (*)(MatchDataBase *);
+
+  mutable std::unique_ptr<MatchDataBase, MatchDataDeleter> MatchData = {
+      nullptr, [](auto *) {}};
+  mutable unsigned MatchDataID = unsigned(-1);
+
 protected:
   CombinerInfo &CInfo;
   GISelChangeObserver &Observer;

llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td

@@ -76,8 +76,15 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
 
 // We have at most 2 registers used by one rule at a time, so we should only have 2 registers MDInfos.
 
-// CHECK:      struct MatchInfosTy {
-// CHECK-NEXT:   Register MDInfo0, MDInfo1;
+// CHECK:      struct MatchDataRule2 : MatchDataBase {
+// CHECK-NEXT:   static constexpr unsigned ID = 2;
+// CHECK-NEXT:   Register   r0;
+// CHECK-NEXT:   Register   r1;
+// CHECK-NEXT: };
+
+// CHECK:      struct MatchDataRule3 : MatchDataBase {
+// CHECK-NEXT:   static constexpr unsigned ID = 3;
+// CHECK-NEXT:   Register   r0;
 // CHECK-NEXT: };
 
 // Check predicates
@@ -96,13 +103,9 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
 // CHECK-NEXT:   B.setInstrAndDebugLoc(I);
 // CHECK-NEXT:   State.MIs.clear();
 // CHECK-NEXT:   State.MIs.push_back(&I);
-// CHECK-NEXT:   MatchInfos = MatchInfosTy();
-// CHECK-EMPTY:
-// CHECK-NEXT:   if (executeMatchTable(*this, State, ExecInfo, B, getMatchTable(), *ST.getInstrInfo(), MRI, *MRI.getTargetRegisterInfo(), *ST.getRegBankInfo(), AvailableFeatures, /*CoverageInfo*/ nullptr))
-// CHECK-NEXT:     return true;
-// CHECK-NEXT:   }
-// CHECK-EMPTY:
-// CHECK-NEXT:   return false;
+// CHECK-NEXT:   const bool Result = executeMatchTable(*this, State, ExecInfo, B, getMatchTable(), *ST.getInstrInfo(), MRI, *MRI.getTargetRegisterInfo(), *ST.getRegBankInfo(), AvailableFeatures, /*CoverageInfo*/ nullptr);
+// CHECK-NEXT:   resetMatchData();
+// CHECK-NEXT:   return Result;
 // CHECK-NEXT: }
 
 // Check apply
@@ -120,7 +123,7 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
 // CHECK-NEXT:   }
 // CHECK-NEXT:   case GICXXCustomAction_CombineApplyGICombiner1:{
 // CHECK-NEXT:     Helper.getBuilder().setInstrAndDebugLoc(*State.MIs[0]);
-// CHECK-NEXT:     APPLY MatchInfos.MDInfo0, MatchInfos.MDInfo1
+// CHECK-NEXT:     APPLY getOrCreateMatchData<MatchDataRule2>().r0, getOrCreateMatchData<MatchDataRule2>().r1
 // CHECK-NEXT:     return;
 // CHECK-NEXT:   }
 // CHECK-NEXT:   case GICXXCustomAction_CombineApplyGICombiner2:{

llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td

@@ -64,8 +64,8 @@ def InstTest0 : GICombineRule<
   (apply [{ APPLY }])>;
 
 // CHECK:      (CombineRule name:InstTest1 id:3 root:d
-// CHECK-NEXT:   (MatchDatas
-// CHECK-NEXT:      (MatchDataInfo pattern_symbol:r0 type:'Register' var_name:MDInfo0)
+// CHECK-NEXT:   (MatchData typename:MatchDataRule3 id:3
+// CHECK-NEXT:     (MatchDataDef symbol:r0 type:Register)
 // CHECK-NEXT:   )
 // CHECK-NEXT:   (MatchPats
 // CHECK-NEXT:     <match_root>d:(CodeGenInstructionPattern COPY operands:[<def>$a, i32:$b])
@@ -89,8 +89,8 @@ def InstTest1 : GICombineRule<
   (apply [{ APPLY }])>;
 
 // CHECK:      (CombineRule name:InstTest2 id:4 root:d
-// CHECK-NEXT:   (MatchDatas
-// CHECK-NEXT:     (MatchDataInfo pattern_symbol:r0 type:'Register' var_name:MDInfo0)
+// CHECK-NEXT:   (MatchData typename:MatchDataRule4 id:4
+// CHECK-NEXT:     (MatchDataDef symbol:r0 type:Register)
 // CHECK-NEXT:   )
 // CHECK-NEXT:   (MatchPats
 // CHECK-NEXT:     <match_root>__InstTest2_match_0:(CodeGenInstructionPattern COPY operands:[<def>$d, (i32 0):$x])

llvm/utils/TableGen/Common/CMakeLists.txt

@@ -16,7 +16,6 @@ add_llvm_library(LLVMTableGenCommon STATIC OBJECT EXCLUDE_FROM_ALL
   GlobalISel/CXXPredicates.cpp
   GlobalISel/GlobalISelMatchTable.cpp
   GlobalISel/GlobalISelMatchTableExecutorEmitter.cpp
-  GlobalISel/MatchDataInfo.cpp
   GlobalISel/PatternParser.cpp
   GlobalISel/Patterns.cpp