Skip to content

Commit 02bcc86

Browse files
committed
[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
1 parent aa89c5f commit 02bcc86

File tree

3 files changed

+53
-4
lines changed

3 files changed

+53
-4
lines changed

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
931931
for (unsigned j = 1; j < MI.getNumOperands(); j += 2)
932932
MIB.addUse(SrcRegs[j / 2][i]).add(MI.getOperand(j + 1));
933933
}
934-
MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
934+
MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
935935
MIRBuilder.buildMerge(MI.getOperand(0).getReg(), DstRegs);
936936
Observer.changedInstr(MI);
937937
MI.eraseFromParent();
@@ -1765,7 +1765,7 @@ LegalizerHelper::widenScalar(MachineInstr &MI, unsigned TypeIdx, LLT WideTy) {
17651765
}
17661766

17671767
MachineBasicBlock &MBB = *MI.getParent();
1768-
MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
1768+
MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
17691769
widenScalarDst(MI, WideTy);
17701770
Observer.changedInstr(MI);
17711771
return Legalized;
@@ -3156,7 +3156,7 @@ LegalizerHelper::moreElementsVectorPhi(MachineInstr &MI, unsigned TypeIdx,
31563156
}
31573157

31583158
MachineBasicBlock &MBB = *MI.getParent();
3159-
MIRBuilder.setInsertPt(MBB, --MBB.getFirstNonPHI());
3159+
MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
31603160
moreElementsVectorDst(MI, MoreTy, 0);
31613161
Observer.changedInstr(MI);
31623162
return Legalized;

llvm/test/CodeGen/AArch64/GlobalISel/legalize-phi.mir

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040
ret i32 0
4141
}
4242

43+
define i32 @legalize_phi_check_insertpt(i64 %a) {
44+
entry:
45+
ret i32 0
46+
}
47+
4348
...
4449
---
4550
name: legalize_phi
@@ -596,3 +601,47 @@ body: |
596601
RET_ReallyLR implicit $w0
597602
598603
...
604+
---
605+
name: legalize_phi_check_insertpt
606+
alignment: 4
607+
exposesReturnsTwice: false
608+
legalized: false
609+
regBankSelected: false
610+
selected: false
611+
tracksRegLiveness: true
612+
body: |
613+
; Check that the G_MERGE here gets inserted after all the PHIs.
614+
; CHECK-LABEL: name: legalize_phi_check_insertpt
615+
; CHECK: bb.0:
616+
; CHECK: successors: %bb.1(0x80000000)
617+
; CHECK: liveins: $x0, $x1
618+
; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
619+
; CHECK: [[COPY1:%[0-9]+]]:_(p0) = COPY $x1
620+
; CHECK: [[DEF:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
621+
; CHECK: [[DEF1:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
622+
; CHECK: G_BR %bb.1
623+
; CHECK: bb.1:
624+
; CHECK: [[PHI:%[0-9]+]]:_(s64) = G_PHI [[DEF]](s64), %bb.0
625+
; CHECK: [[PHI1:%[0-9]+]]:_(s64) = G_PHI [[DEF1]](s64), %bb.0
626+
; CHECK: [[PHI2:%[0-9]+]]:_(s64) = G_PHI [[COPY]](s64), %bb.0
627+
; CHECK: [[MV:%[0-9]+]]:_(s128) = G_MERGE_VALUES [[PHI]](s64), [[PHI1]](s64)
628+
; CHECK: G_STORE [[MV]](s128), [[COPY1]](p0) :: (store 16)
629+
; CHECK: G_STORE [[PHI2]](s64), [[COPY1]](p0) :: (store 8)
630+
; CHECK: RET_ReallyLR
631+
bb.0:
632+
successors: %bb.1(0x40000000)
633+
liveins: $x0, $x1
634+
635+
%0:_(s64) = COPY $x0
636+
%1:_(p0) = COPY $x1
637+
%2:_(s128) = G_IMPLICIT_DEF
638+
G_BR %bb.1
639+
640+
bb.1:
641+
%3:_(s128) = G_PHI %2(s128), %bb.0
642+
%4:_(s64) = G_PHI %0(s64), %bb.0
643+
G_STORE %3(s128), %1(p0) :: (store 16)
644+
G_STORE %4(s64), %1(p0) :: (store 8)
645+
RET_ReallyLR
646+
647+
...

llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ body: |
160160
; CHECK: G_BR %bb.2
161161
; CHECK: bb.2:
162162
; CHECK: [[PHI:%[0-9]+]]:_(<4 x s16>) = G_PHI [[INSERT]](<4 x s16>), %bb.0, [[INSERT3]](<4 x s16>), %bb.1
163-
; CHECK: [[EXTRACT1:%[0-9]+]]:_(<3 x s16>) = G_EXTRACT [[PHI]](<4 x s16>), 0
164163
; CHECK: [[DEF4:%[0-9]+]]:_(<4 x s16>) = G_IMPLICIT_DEF
164+
; CHECK: [[EXTRACT1:%[0-9]+]]:_(<3 x s16>) = G_EXTRACT [[PHI]](<4 x s16>), 0
165165
; CHECK: [[INSERT4:%[0-9]+]]:_(<4 x s16>) = G_INSERT [[DEF4]], [[EXTRACT1]](<3 x s16>), 0
166166
; CHECK: $vgpr0_vgpr1 = COPY [[INSERT4]](<4 x s16>)
167167
; CHECK: S_SETPC_B64 undef $sgpr30_sgpr31

0 commit comments

Comments
 (0)