From 02bcc86b08dc1f50be401240307d6819483ca822 Mon Sep 17 00:00:00 2001
From: Amara Emerson <aemerson@apple.com>
Date: Fri, 13 Sep 2019 21:49:24 +0000
Subject: [PATCH] [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

llvm-svn: 371901
---
 llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp    |  6 +--
 .../CodeGen/AArch64/GlobalISel/legalize-phi.mir    | 49 ++++++++++++++++++++++
 .../CodeGen/AMDGPU/GlobalISel/legalize-phi.mir     |  2 +-
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index 9239576..8c356c6 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -931,7 +931,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
       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(MachineInstr &MI, unsigned TypeIdx, LLT WideTy) {
     }
 
     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(MachineInstr &MI, unsigned TypeIdx,
   }
 
   MachineBasicBlock &MBB = *MI.getParent();
-  MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
+  MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
   moreElementsVectorDst(MI, MoreTy, 0);
   Observer.changedInstr(MI);
   return Legalized;
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir
index a92b064..1c210a4 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir
@@ -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
+
+...
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir
index f17323c..24b144c 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir
@@ -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
-- 
2.7.4