[llvm] 507d193 - [AArch64][GlobalISel] Handle multiple phis in fixupPHIOpBanks
Jessica Paquette via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 4 09:59:57 PDT 2021
Author: Jessica Paquette
Date: 2021-06-04T09:59:36-07:00
New Revision: 507d193ea7ef20ec77615b55836b4653f736cef3
URL: https://github.com/llvm/llvm-project/commit/507d193ea7ef20ec77615b55836b4653f736cef3
DIFF: https://github.com/llvm/llvm-project/commit/507d193ea7ef20ec77615b55836b4653f736cef3.diff
LOG: [AArch64][GlobalISel] Handle multiple phis in fixupPHIOpBanks
If we ended up with two phi instructions in a block, and we needed to fix up
the banks for the first one, we'd end up inserting our COPY before the second
phi.
E.g.
```
%x = G_PHI ...
%fixup = COPY ...
%y = G_PHI ...
```
This is invalid MIR, and breaks assumptions made by the register allocator later
down the line. With the verifier enabled, it also emits a verification error.
This teaches fixupPHIOpBanks to walk past any phi instructions in the block
when emitting the fixup copies.
Here's an example of the crashing code (same as added testcase):
https://godbolt.org/z/h5j1x3o6e
Differential Revision: https://reviews.llvm.org/D103582
Added:
Modified:
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
llvm/test/CodeGen/AArch64/GlobalISel/preselect-process-phis.mir
Removed:
################################################################################
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 9c58fd590909..9a17435f5e09 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -6018,7 +6018,14 @@ static void fixupPHIOpBanks(MachineInstr &MI, MachineRegisterInfo &MRI,
// Insert a cross-bank copy.
auto *OpDef = MRI.getVRegDef(OpReg);
const LLT &Ty = MRI.getType(OpReg);
- MIB.setInsertPt(*OpDef->getParent(), std::next(OpDef->getIterator()));
+ MachineBasicBlock &OpDefBB = *OpDef->getParent();
+
+ // Any instruction we insert must appear after all PHIs in the block
+ // for the block to be valid MIR.
+ MachineBasicBlock::iterator InsertPt = std::next(OpDef->getIterator());
+ if (InsertPt != OpDefBB.end() && InsertPt->isPHI())
+ InsertPt = OpDefBB.getFirstNonPHI();
+ MIB.setInsertPt(*OpDef->getParent(), InsertPt);
auto Copy = MIB.buildCopy(Ty, OpReg);
MRI.setRegBank(Copy.getReg(0), *DstRB);
MO.setReg(Copy.getReg(0));
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/preselect-process-phis.mir b/llvm/test/CodeGen/AArch64/GlobalISel/preselect-process-phis.mir
index c427d8004d9a..01d55def986b 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/preselect-process-phis.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/preselect-process-phis.mir
@@ -108,3 +108,88 @@ body: |
G_BR %bb.2
...
+---
+name: multiple_phis
+alignment: 4
+legalized: true
+regBankSelected: true
+tracksRegLiveness: true
+body: |
+
+ ; The copy we insert in bb.4 should appear after all the phi instructions.
+
+ ; CHECK-LABEL: name: multiple_phis
+ ; CHECK: bb.0:
+ ; CHECK: successors: %bb.1(0x40000000), %bb.5(0x40000000)
+ ; CHECK: liveins: $w0, $w1, $x2
+ ; CHECK: %ptr:gpr64sp = COPY $x2
+ ; CHECK: %cond_1:gpr32 = IMPLICIT_DEF
+ ; CHECK: %gpr_1:gpr32 = IMPLICIT_DEF
+ ; CHECK: [[COPY:%[0-9]+]]:fpr32 = COPY %gpr_1
+ ; CHECK: [[COPY1:%[0-9]+]]:fpr16 = COPY [[COPY]].hsub
+ ; CHECK: TBNZW %cond_1, 0, %bb.5
+ ; CHECK: B %bb.1
+ ; CHECK: bb.1:
+ ; CHECK: successors: %bb.2(0x40000000), %bb.3(0x40000000)
+ ; CHECK: %cond_2:gpr32 = IMPLICIT_DEF
+ ; CHECK: TBNZW %cond_2, 0, %bb.3
+ ; CHECK: B %bb.2
+ ; CHECK: bb.2:
+ ; CHECK: successors: %bb.4(0x80000000)
+ ; CHECK: %gpr_2:gpr32 = IMPLICIT_DEF
+ ; CHECK: B %bb.4
+ ; CHECK: bb.3:
+ ; CHECK: successors: %bb.4(0x80000000)
+ ; CHECK: %fpr:fpr16 = IMPLICIT_DEF
+ ; CHECK: bb.4:
+ ; CHECK: successors: %bb.5(0x80000000)
+ ; CHECK: %fp_phi:fpr16 = PHI %fpr, %bb.3, [[COPY1]], %bb.2
+ ; CHECK: %gp_phi1:gpr32 = PHI %gpr_1, %bb.3, %gpr_2, %bb.2
+ ; CHECK: %gp_phi2:gpr32 = PHI %gpr_1, %bb.3, %gpr_2, %bb.2
+ ; CHECK: %gp_phi3:gpr32 = PHI %gpr_1, %bb.3, %gpr_2, %bb.2
+ ; CHECK: [[SUBREG_TO_REG:%[0-9]+]]:fpr32 = SUBREG_TO_REG 0, %fp_phi, %subreg.hsub
+ ; CHECK: [[COPY2:%[0-9]+]]:gpr32all = COPY [[SUBREG_TO_REG]]
+ ; CHECK: bb.5:
+ ; CHECK: %use_fp_phi:gpr32 = PHI %gpr_1, %bb.0, [[COPY2]], %bb.4
+ ; CHECK: %use_gp_phi1:gpr32 = PHI %gpr_1, %bb.0, %gp_phi1, %bb.4
+ ; CHECK: %use_gp_phi2:gpr32 = PHI %gpr_1, %bb.0, %gp_phi2, %bb.4
+ ; CHECK: %use_gp_phi3:gpr32 = PHI %gpr_1, %bb.0, %gp_phi3, %bb.4
+ ; CHECK: STRHHui %use_fp_phi, %ptr, 0 :: (store 2)
+ ; CHECK: STRHHui %use_gp_phi1, %ptr, 0 :: (store 2)
+ ; CHECK: STRHHui %use_gp_phi2, %ptr, 0 :: (store 2)
+ ; CHECK: STRHHui %use_gp_phi3, %ptr, 0 :: (store 2)
+ ; CHECK: RET_ReallyLR
+ bb.1:
+ successors: %bb.2, %bb.6
+ liveins: $w0, $w1, $x2
+ %ptr:gpr(p0) = COPY $x2
+ %cond_1:gpr(s1) = G_IMPLICIT_DEF
+ %gpr_1:gpr(s16) = G_IMPLICIT_DEF
+ G_BRCOND %cond_1(s1), %bb.6
+ G_BR %bb.2
+ bb.2:
+ successors: %bb.3, %bb.4
+ %cond_2:gpr(s1) = G_IMPLICIT_DEF
+ G_BRCOND %cond_2(s1), %bb.4
+ G_BR %bb.3
+ bb.3:
+ %gpr_2:gpr(s16) = G_IMPLICIT_DEF
+ G_BR %bb.5
+ bb.4:
+ %fpr:fpr(s16) = G_IMPLICIT_DEF
+ bb.5:
+ %fp_phi:fpr(s16) = G_PHI %fpr(s16), %bb.4, %gpr_1(s16), %bb.3
+ %gp_phi1:gpr(s16) = G_PHI %gpr_1(s16), %bb.4, %gpr_2(s16), %bb.3
+ %gp_phi2:gpr(s16) = G_PHI %gpr_1(s16), %bb.4, %gpr_2(s16), %bb.3
+ %gp_phi3:gpr(s16) = G_PHI %gpr_1(s16), %bb.4, %gpr_2(s16), %bb.3
+ bb.6:
+ %use_fp_phi:gpr(s16) = G_PHI %gpr_1(s16), %bb.1, %fp_phi(s16), %bb.5
+ %use_gp_phi1:gpr(s16) = G_PHI %gpr_1(s16), %bb.1, %gp_phi1(s16), %bb.5
+ %use_gp_phi2:gpr(s16) = G_PHI %gpr_1(s16), %bb.1, %gp_phi2(s16), %bb.5
+ %use_gp_phi3:gpr(s16) = G_PHI %gpr_1(s16), %bb.1, %gp_phi3(s16), %bb.5
+ G_STORE %use_fp_phi(s16), %ptr(p0) :: (store 2)
+ G_STORE %use_gp_phi1(s16), %ptr(p0) :: (store 2)
+ G_STORE %use_gp_phi2(s16), %ptr(p0) :: (store 2)
+ G_STORE %use_gp_phi3(s16), %ptr(p0) :: (store 2)
+ RET_ReallyLR
+...
More information about the llvm-commits
mailing list