[GlobalISel] Fix D144336 in a different way, by choosing operands fro…

…m the first of the div/rem insts.

Differential Revision: https://reviews.llvm.org/D144336

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

@@ -1113,7 +1113,6 @@ void CombinerHelper::applyCombineIndexedLoadStore(
 
 bool CombinerHelper::matchCombineDivRem(MachineInstr &MI,
                                         MachineInstr *&OtherMI) {
-  OtherMI = nullptr;
   unsigned Opcode = MI.getOpcode();
   bool IsDiv, IsSigned;
 
@@ -1168,44 +1167,11 @@ bool CombinerHelper::matchCombineDivRem(MachineInstr &MI,
         matchEqualDefs(MI.getOperand(2), UseMI.getOperand(2)) &&
         matchEqualDefs(MI.getOperand(1), UseMI.getOperand(1))) {
       OtherMI = &UseMI;
-      break;
+      return true;
     }
   }
-  if (!OtherMI)
-    return false;
-
-  // We may have a situation like this:
-  //   %4:_(s32) = G_SEXT %2:_(s1)
-  //   %5:_(s32) = G_SEXT %2:_(s1)
-  //   %6:_(s32) = G_UDIV %4:_, %5:_
-  //   %8:_(s32) = G_SEXT %2:_(s1)
-  //   %9:_(s32) = G_UREM %5:_, %8:_
-  // and choosing the insertion point as the G_UDIV will cause it to use %8
-  // before the def. We check here if any of the operands of the later
-  // instruction (i.e. one of DIV/REM that is the second in the block) are
-  // dominated by the first instruction. In this case we check if %8 is
-  // dominated by the G_UDIV and bail out if so.
 
-  SmallSet<Register, 2> RegsToCheck;
-  MachineInstr *First, *Second;
-  if (dominates(MI, *OtherMI)) {
-    First = &MI;
-    Second = OtherMI;
-  } else {
-    First = OtherMI;
-    Second = &MI;
-  }
-  RegsToCheck.insert(Second->getOperand(1).getReg());
-  RegsToCheck.insert(Second->getOperand(2).getReg());
-  for (MachineBasicBlock::iterator II = std::next(First->getIterator());
-       II != Second->getIterator(); ++II) {
-    for (auto &MO : II->operands()) {
-      if (MO.isReg() && MO.isDef() && RegsToCheck.count(MO.getReg()) &&
-          dominates(*First, *II))
-        return false;
-    }
-  }
-  return true;
+  return false;
 }
 
 void CombinerHelper::applyCombineDivRem(MachineInstr &MI,
@@ -1226,16 +1192,22 @@ void CombinerHelper::applyCombineDivRem(MachineInstr &MI,
       Opcode == TargetOpcode::G_SDIV || Opcode == TargetOpcode::G_SREM;
 
   // Check which instruction is first in the block so we don't break def-use
-  // deps by "moving" the instruction incorrectly.
-  if (dominates(MI, *OtherMI))
+  // deps by "moving" the instruction incorrectly. Also keep track of which
+  // instruction is first so we pick it's operands, avoiding use-before-def
+  // bugs.
+  MachineInstr *FirstInst;
+  if (dominates(MI, *OtherMI)) {
     Builder.setInstrAndDebugLoc(MI);
-  else
+    FirstInst = &MI;
+  } else {
     Builder.setInstrAndDebugLoc(*OtherMI);
+    FirstInst = OtherMI;
+  }
 
   Builder.buildInstr(IsSigned ? TargetOpcode::G_SDIVREM
                               : TargetOpcode::G_UDIVREM,
                      {DestDivReg, DestRemReg},
-                     {MI.getOperand(1).getReg(), MI.getOperand(2).getReg()});
+                     { FirstInst->getOperand(1), FirstInst->getOperand(2) });
   MI.eraseFromParent();
   OtherMI->eraseFromParent();
 }

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-conflict.mir

@@ -1,7 +1,6 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - | FileCheck %s
 
-# Check that we don't combine to G_UDIVREM if it would cause a use-before-def
 ---
 name:            test
 alignment:       4
@@ -12,13 +11,11 @@ body:             |
     ; CHECK: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
     ; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
     ; CHECK-NEXT: [[SEXT1:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
-    ; CHECK-NEXT: [[UDIV:%[0-9]+]]:_(s32) = G_UDIV [[SEXT]], [[SEXT1]]
-    ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
-    ; CHECK-NEXT: [[UREM:%[0-9]+]]:_(s32) = G_UREM [[SEXT1]], [[SEXT2]]
-    ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UREM]](s32)
-    ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIV]](s32)
-    ; CHECK-NEXT: [[SEXT3:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
-    ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT3]]
+    ; CHECK-NEXT: [[UDIVREM:%[0-9]+]]:_(s32), [[UDIVREM1:%[0-9]+]]:_ = G_UDIVREM [[SEXT]], [[SEXT1]]
+    ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIVREM1]](s32)
+    ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIVREM]](s32)
+    ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
+    ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT2]]
     ; CHECK-NEXT: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[OR]](s64)
     ; CHECK-NEXT: $w0 = COPY [[TRUNC1]](s32)
     ; CHECK-NEXT: RET_ReallyLR implicit $w0
@@ -53,48 +50,9 @@ body:             |
     ; CHECK: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
     ; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
     ; CHECK-NEXT: [[SEXT1:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
-    ; CHECK-NEXT: [[UREM:%[0-9]+]]:_(s32) = G_UREM [[SEXT]], [[SEXT1]]
-    ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
-    ; CHECK-NEXT: [[UDIV:%[0-9]+]]:_(s32) = G_UDIV [[SEXT1]], [[SEXT2]]
-    ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIV]](s32)
-    ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UREM]](s32)
-    ; CHECK-NEXT: [[SEXT3:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
-    ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT3]]
-    ; CHECK-NEXT: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[OR]](s64)
-    ; CHECK-NEXT: $w0 = COPY [[TRUNC1]](s32)
-    ; CHECK-NEXT: RET_ReallyLR implicit $w0
-    %0:_(s16) = G_CONSTANT i16 0
-    %2:_(s1) = G_CONSTANT i1 true
-    %1:_(s1) = G_ICMP intpred(sge), %0(s16), %0
-    %3:_(s8) = G_SEXT %2(s1)
-    %4:_(s32) = G_SEXT %3(s8)
-    %5:_(s32) = G_SEXT %1(s1)
-    %6:_(s32) = G_UREM %4, %5
-    %7:_(s32) = COPY %5(s32)
-    %8:_(s32) = G_SEXT %2(s1)
-    %9:_(s32) = G_UDIV %7, %8
-    %10:_(s8) = G_TRUNC %9(s32)
-    %11:_(s64) = G_ZEXT %6(s32)
-    %12:_(s64) = G_SEXT %10(s8)
-    %13:_(s64) = G_OR %11, %12
-    %14:_(s32) = G_TRUNC %13(s64)
-    $w0 = COPY %14(s32)
-    RET_ReallyLR implicit $w0
-
-...
----
-name:            ok_before_first
-alignment:       4
-tracksRegLiveness: true
-body:             |
-  bb.1:
-    ; CHECK-LABEL: name: ok_before_first
-    ; CHECK: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
-    ; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
-    ; CHECK-NEXT: [[SEXT1:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
     ; CHECK-NEXT: [[UDIVREM:%[0-9]+]]:_(s32), [[UDIVREM1:%[0-9]+]]:_ = G_UDIVREM [[SEXT]], [[SEXT1]]
-    ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIVREM1]](s32)
-    ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIVREM]](s32)
+    ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIVREM]](s32)
+    ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIVREM1]](s32)
     ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
     ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT2]]
     ; CHECK-NEXT: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[OR]](s64)
@@ -106,10 +64,10 @@ body:             |
     %3:_(s8) = G_SEXT %2(s1)
     %4:_(s32) = G_SEXT %3(s8)
     %5:_(s32) = G_SEXT %1(s1)
-    %8:_(s32) = G_SEXT %2(s1)
-    %6:_(s32) = G_UDIV %4, %5
+    %6:_(s32) = G_UREM %4, %5
     %7:_(s32) = COPY %5(s32)
-    %9:_(s32) = G_UREM %7, %8
+    %8:_(s32) = G_SEXT %2(s1)
+    %9:_(s32) = G_UDIV %7, %8
     %10:_(s8) = G_TRUNC %9(s32)
     %11:_(s64) = G_ZEXT %6(s32)
     %12:_(s64) = G_SEXT %10(s8)