[llvm] r371901 - [GlobalISel] Fix insertion point of new instructions to be after PHIs.

[Amara Emerson via llvm-commits]

Author: aemerson
Date: Fri Sep 13 14:49:24 2019
New Revision: 371901

URL: http://llvm.org/viewvc/llvm-project?rev=371901&view=rev
Log:
[GlobalISel] Fix insertion point of new instructions to be after PHIs.

For some reason we sometimes insert new instructions one instruction before
the first non-PHI when legalizing. This can result in having non-PHI
instructions before PHIs, which mean that PHI elimination doesn't catch them.

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

Modified:
    llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir
    llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir

Modified: llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp?rev=371901&r1=371900&r2=371901&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Fri Sep 13 14:49:24 2019
@@ -931,7 +931,7 @@ LegalizerHelper::LegalizeResult Legalize
       for (unsigned j = 1; j < MI.getNumOperands(); j += 2)
         MIB.addUse(SrcRegs[j / 2][i]).add(MI.getOperand(j + 1));
     }
-    MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
+    MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
     MIRBuilder.buildMerge(MI.getOperand(0).getReg(), DstRegs);
     Observer.changedInstr(MI);
     MI.eraseFromParent();
@@ -1765,7 +1765,7 @@ LegalizerHelper::widenScalar(MachineInst
     }
 
     MachineBasicBlock &MBB = *MI.getParent();
-    MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
+    MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
     widenScalarDst(MI, WideTy);
     Observer.changedInstr(MI);
     return Legalized;
@@ -3156,7 +3156,7 @@ LegalizerHelper::moreElementsVectorPhi(M
   }
 
   MachineBasicBlock &MBB = *MI.getParent();
-  MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
+  MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
   moreElementsVectorDst(MI, MoreTy, 0);
   Observer.changedInstr(MI);
   return Legalized;

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir?rev=371901&r1=371900&r2=371901&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir Fri Sep 13 14:49:24 2019
@@ -40,6 +40,11 @@
     ret i32 0
   }
 
+  define i32 @legalize_phi_check_insertpt(i64 %a) {
+  entry:
+    ret i32 0
+  }
+
 ...
 ---
 name:            legalize_phi
@@ -596,3 +601,47 @@ body:             |
     RET_ReallyLR implicit $w0
 
 ...
+---
+name:            legalize_phi_check_insertpt
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+body:             |
+  ; Check that the G_MERGE here gets inserted after all the PHIs.
+  ; CHECK-LABEL: name: legalize_phi_check_insertpt
+  ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   liveins: $x0, $x1
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s64) = COPY $x0
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
+  ; CHECK:   [[DEF:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
+  ; CHECK:   [[DEF1:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
+  ; CHECK:   G_BR %bb.1
+  ; CHECK: bb.1:
+  ; CHECK:   [[PHI:%[0-9]+]]:_(s64) = G_PHI [[DEF]](s64), %bb.0
+  ; CHECK:   [[PHI1:%[0-9]+]]:_(s64) = G_PHI [[DEF1]](s64), %bb.0
+  ; CHECK:   [[PHI2:%[0-9]+]]:_(s64) = G_PHI [[COPY]](s64), %bb.0
+  ; CHECK:   [[MV:%[0-9]+]]:_(s128) = G_MERGE_VALUES [[PHI]](s64), [[PHI1]](s64)
+  ; CHECK:   G_STORE [[MV]](s128), [[COPY1]](p0) :: (store 16)
+  ; CHECK:   G_STORE [[PHI2]](s64), [[COPY1]](p0) :: (store 8)
+  ; CHECK:   RET_ReallyLR
+  bb.0:
+    successors: %bb.1(0x40000000)
+    liveins: $x0, $x1
+
+    %0:_(s64) = COPY $x0
+    %1:_(p0) = COPY $x1
+    %2:_(s128) = G_IMPLICIT_DEF
+    G_BR %bb.1
+
+  bb.1:
+    %3:_(s128) = G_PHI %2(s128), %bb.0
+    %4:_(s64) = G_PHI %0(s64), %bb.0
+    G_STORE %3(s128), %1(p0) :: (store 16)
+    G_STORE %4(s64), %1(p0) :: (store 8)
+    RET_ReallyLR
+
+...

Modified: llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir?rev=371901&r1=371900&r2=371901&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir (original)
+++ llvm/trunk/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir Fri Sep 13 14:49:24 2019
@@ -160,8 +160,8 @@ body: |
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
   ; CHECK:   [[PHI:%[0-9]+]]:_(<4 x s16>) = G_PHI [[INSERT]](<4 x s16>), %bb.0, [[INSERT3]](<4 x s16>), %bb.1
-  ; CHECK:   [[EXTRACT1:%[0-9]+]]:_(<3 x s16>) = G_EXTRACT [[PHI]](<4 x s16>), 0
   ; CHECK:   [[DEF4:%[0-9]+]]:_(<4 x s16>) = G_IMPLICIT_DEF
+  ; CHECK:   [[EXTRACT1:%[0-9]+]]:_(<3 x s16>) = G_EXTRACT [[PHI]](<4 x s16>), 0
   ; CHECK:   [[INSERT4:%[0-9]+]]:_(<4 x s16>) = G_INSERT [[DEF4]], [[EXTRACT1]](<3 x s16>), 0
   ; CHECK:   $vgpr0_vgpr1 = COPY [[INSERT4]](<4 x s16>)
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31