From f984b0e177f8342a66bde66f65c71ac68bc9acf0 Mon Sep 17 00:00:00 2001
From: Amara Emerson <amara@apple.com>
Date: Thu, 29 Jul 2021 17:07:19 -0700
Subject: [PATCH] [GlobalISel] Refactor the unmerge artifact value finder code.

I moved the code that tries to combine away each unmerge def into a method in
ArtifactValueFinder class itself. This removes a logically messy lambda and
makes it easier to use the value-finder in more places in future.
---
 .../GlobalISel/LegalizationArtifactCombiner.h      | 71 +++++++++++++---------
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
index 44a4892..1950193 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
@@ -15,6 +15,7 @@
 #define LLVM_CODEGEN_GLOBALISEL_LEGALIZATIONARTIFACTCOMBINER_H
 
 #include "llvm/ADT/SmallBitVector.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
 #include "llvm/CodeGen/GlobalISel/Legalizer.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
@@ -743,38 +744,26 @@ public:
         return Register();
       }
     }
-  };
 
-  bool tryCombineUnmergeValues(GUnmerge &MI,
-                               SmallVectorImpl<MachineInstr *> &DeadInsts,
-                               SmallVectorImpl<Register> &UpdatedDefs,
-                               GISelChangeObserver &Observer) {
-    unsigned NumDefs = MI.getNumDefs();
-    Register SrcReg = MI.getSourceReg();
-    MachineInstr *SrcDef = getDefIgnoringCopies(SrcReg, MRI);
-    if (!SrcDef)
-      return false;
-
-    LLT OpTy = MRI.getType(SrcReg);
-    LLT DestTy = MRI.getType(MI.getReg(0));
-    unsigned SrcDefIdx = getDefIndex(*SrcDef, SrcReg);
-
-    Builder.setInstrAndDebugLoc(MI);
-
-    auto tryCombineViaValueFinder = [&]() {
-      ArtifactValueFinder ValueFinder(MRI, Builder, LI);
+    /// Try to combine the defs of an unmerge \p MI by attempting to find
+    /// values that provides the bits for each def reg.
+    /// \returns true if all the defs of the unmerge have been made dead.
+    bool tryCombineUnmergeDefs(GUnmerge &MI, GISelChangeObserver &Observer,
+                               SmallVectorImpl<Register> &UpdatedDefs) {
+      unsigned NumDefs = MI.getNumDefs();
+      LLT DestTy = MRI.getType(MI.getReg(0));
 
       SmallBitVector DeadDefs(NumDefs);
       for (unsigned DefIdx = 0; DefIdx < NumDefs; ++DefIdx) {
         Register DefReg = MI.getReg(DefIdx);
         Register FoundVal =
-            ValueFinder.findValueFromDef(DefReg, 0, DestTy.getSizeInBits());
+            findValueFromDef(DefReg, 0, DestTy.getSizeInBits());
         if (!FoundVal || FoundVal == DefReg)
           continue;
         if (MRI.getType(FoundVal) != DestTy)
           continue;
 
-        replaceRegOrBuildCopy(DefReg, FoundVal, MRI, Builder, UpdatedDefs,
+        replaceRegOrBuildCopy(DefReg, FoundVal, MRI, MIB, UpdatedDefs,
                               Observer);
         // We only want to replace the uses, not the def of the old reg.
         Observer.changingInstr(MI);
@@ -782,12 +771,25 @@ public:
         Observer.changedInstr(MI);
         DeadDefs[DefIdx] = true;
       }
-      if (DeadDefs.all()) {
-        markInstAndDefDead(MI, *SrcDef, DeadInsts, SrcDefIdx);
-        return true;
-      }
+      return DeadDefs.all();
+    }
+  };
+
+  bool tryCombineUnmergeValues(GUnmerge &MI,
+                               SmallVectorImpl<MachineInstr *> &DeadInsts,
+                               SmallVectorImpl<Register> &UpdatedDefs,
+                               GISelChangeObserver &Observer) {
+    unsigned NumDefs = MI.getNumDefs();
+    Register SrcReg = MI.getSourceReg();
+    MachineInstr *SrcDef = getDefIgnoringCopies(SrcReg, MRI);
+    if (!SrcDef)
       return false;
-    };
+
+    LLT OpTy = MRI.getType(SrcReg);
+    LLT DestTy = MRI.getType(MI.getReg(0));
+    unsigned SrcDefIdx = getDefIndex(*SrcDef, SrcReg);
+
+    Builder.setInstrAndDebugLoc(MI);
 
     if (auto *SrcUnmerge = dyn_cast<GUnmerge>(SrcDef)) {
       // %0:_(<4 x s16>) = G_FOO
@@ -812,8 +814,14 @@ public:
         if (ActionStep.TypeIdx == 1)
           return false;
         break;
-      default:
-        return tryCombineViaValueFinder();
+      default: {
+        ArtifactValueFinder Finder(MRI, Builder, LI);
+        if (Finder.tryCombineUnmergeDefs(MI, Observer, UpdatedDefs)) {
+          markInstAndDefDead(MI, *SrcDef, DeadInsts, SrcDefIdx);
+          return true;
+        }
+        return false;
+      }
       }
 
       auto NewUnmerge = Builder.buildUnmerge(DestTy, SrcUnmergeSrc);
@@ -849,7 +857,12 @@ public:
         return true;
 
       // Try using the value finder.
-      return tryCombineViaValueFinder();
+      ArtifactValueFinder Finder(MRI, Builder, LI);
+      if (Finder.tryCombineUnmergeDefs(MI, Observer, UpdatedDefs)) {
+        markInstAndDefDead(MI, *SrcDef, DeadInsts, SrcDefIdx);
+        return true;
+      }
+      return false;
     }
 
     const unsigned NumMergeRegs = MergeI->getNumOperands() - 1;
-- 
2.7.4