[PATCH] D64513: [GlobalISel][AArch64][NFC] Use getDefIgnoringCopies from Utils where we can

[Jessica Paquette via Phabricator via llvm-commits]

paquette created this revision.
paquette added a reviewer: aemerson.
Herald added subscribers: Petar.Avramovic, hiraditya, kristof.beyls, javed.absar, rovka.
Herald added a project: LLVM.

There are a few places where we walk over copies throughout AArch64InstructionSelector.cpp. In Utils, there's a function that does exactly this which we can use instead.

      

Note that the utility function works with the case where we run into a COPY from a physical register. We've run into bugs with this a couple times, so using it should defend us from similar future bugs.


https://reviews.llvm.org/D64513

Files:
  llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp


Index: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
+++ llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
@@ -2811,13 +2811,10 @@
       "G_SHUFFLE_VECTOR should have a constant mask operand as G_BUILD_VECTOR");
   // Find the constant indices.
   for (unsigned i = 1, e = MaskDef->getNumOperands(); i < e; ++i) {
-    MachineInstr *ScalarDef = MRI.getVRegDef(MaskDef->getOperand(i).getReg());
-    assert(ScalarDef && "Could not find vreg def of shufflevec index op");
     // Look through copies.
-    while (ScalarDef->getOpcode() == TargetOpcode::COPY) {
-      ScalarDef = MRI.getVRegDef(ScalarDef->getOperand(1).getReg());
-      assert(ScalarDef && "Could not find def of copy operand");
-    }
+    MachineInstr *ScalarDef =
+        getDefIgnoringCopies(MaskDef->getOperand(i).getReg(), MRI);
+    assert(ScalarDef && "Could not find vreg def of shufflevec index op");
     if (ScalarDef->getOpcode() != TargetOpcode::G_CONSTANT) {
       // This be an undef if not a constant.
       assert(ScalarDef->getOpcode() == TargetOpcode::G_IMPLICIT_DEF);
@@ -3229,20 +3226,6 @@
   //
   // cmn z, y
 
-  // Helper lambda to find the def.
-  auto FindDef = [&](Register VReg) {
-    MachineInstr *Def = MRI.getVRegDef(VReg);
-    while (Def) {
-      if (Def->getOpcode() != TargetOpcode::COPY)
-        break;
-      // Copies can be from physical registers. If we hit this, we're done.
-      if (TargetRegisterInfo::isPhysicalRegister(Def->getOperand(1).getReg()))
-        break;
-      Def = MRI.getVRegDef(Def->getOperand(1).getReg());
-    }
-    return Def;
-  };
-
   // Helper lambda to detect the subtract followed by the compare.
   // Takes in the def of the LHS or RHS, and checks if it's a subtract from 0.
   auto IsCMN = [&](MachineInstr *DefMI, const AArch64CC::CondCode &CC) {
@@ -3269,8 +3252,8 @@
   };
 
   // Check if the RHS or LHS of the G_ICMP is defined by a SUB
-  MachineInstr *LHSDef = FindDef(LHS.getReg());
-  MachineInstr *RHSDef = FindDef(RHS.getReg());
+  MachineInstr *LHSDef = getDefIgnoringCopies(LHS.getReg(), MRI);
+  MachineInstr *RHSDef = getDefIgnoringCopies(RHS.getReg(), MRI);
   CmpInst::Predicate P = (CmpInst::Predicate)Predicate.getPredicate();
   const AArch64CC::CondCode CC = changeICMPPredToAArch64CC(P);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64513.209003.patch
Type: text/x-patch
Size: 2404 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190710/c065fad9/attachment.bin>