Skip to content

Commit 8848258

Browse files
aeubanksaeubanks
authored
[SROA] Unfold gep of index phi (round 2) (#83494)
If a gep has only one phi as one of its operands and the remaining indexes are constant, we can unfold `gep ptr, (phi idx1, idx2)` to `phi ((gep ptr, idx1), (gep ptr, idx2))`. Take care not to unfold recursive phis. Followup to #80983. This was initially was #83087. Initial PR did not handle allocas in entry block that weren't at the beginning of the function, causing GEPs to be inserted after the first chunk of allocas but potentially before an alloca not at the beginning. Insert GEPs at the end of the entry block instead since constants/arguments/static allocas can all be used there.
1 parent a5b7971 commit 8848258

File tree

3 files changed

+278
-67
lines changed

3 files changed

+278
-67
lines changed

‎llvm/lib/Transforms/Scalar/SROA.cpp

Lines changed: 76 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3956,11 +3956,11 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
39563956
return false;
39573957
}
39583958

3959-
// Fold gep (select cond, ptr1, ptr2), idx
3959+
// Unfold gep (select cond, ptr1, ptr2), idx
39603960
// => select cond, gep(ptr1, idx), gep(ptr2, idx)
39613961
// and gep ptr, (select cond, idx1, idx2)
39623962
// => select cond, gep(ptr, idx1), gep(ptr, idx2)
3963-
bool foldGEPSelect(GetElementPtrInst &GEPI) {
3963+
bool unfoldGEPSelect(GetElementPtrInst &GEPI) {
39643964
// Check whether the GEP has exactly one select operand and all indices
39653965
// will become constant after the transform.
39663966
SelectInst *Sel = dyn_cast<SelectInst>(GEPI.getPointerOperand());
@@ -4029,67 +4029,104 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
40294029
return true;
40304030
}
40314031

4032-
// Fold gep (phi ptr1, ptr2) => phi gep(ptr1), gep(ptr2)
4033-
bool foldGEPPhi(GetElementPtrInst &GEPI) {
4034-
if (!GEPI.hasAllConstantIndices())
4035-
return false;
4032+
// Unfold gep (phi ptr1, ptr2), idx
4033+
// => phi ((gep ptr1, idx), (gep ptr2, idx))
4034+
// and gep ptr, (phi idx1, idx2)
4035+
// => phi ((gep ptr, idx1), (gep ptr, idx2))
4036+
bool unfoldGEPPhi(GetElementPtrInst &GEPI) {
4037+
// To prevent infinitely expanding recursive phis, bail if the GEP pointer
4038+
// operand (looking through the phi if it is the phi we want to unfold) is
4039+
// an instruction besides an alloca.
4040+
PHINode *Phi = dyn_cast<PHINode>(GEPI.getPointerOperand());
4041+
auto IsInvalidPointerOperand = [](Value *V) {
4042+
return isa<Instruction>(V) && !isa<AllocaInst>(V);
4043+
};
4044+
if (Phi) {
4045+
if (any_of(Phi->operands(), IsInvalidPointerOperand))
4046+
return false;
4047+
} else {
4048+
if (IsInvalidPointerOperand(GEPI.getPointerOperand()))
4049+
return false;
4050+
}
4051+
// Check whether the GEP has exactly one phi operand (including the pointer
4052+
// operand) and all indices will become constant after the transform.
4053+
for (Value *Op : GEPI.indices()) {
4054+
if (auto *SI = dyn_cast<PHINode>(Op)) {
4055+
if (Phi)
4056+
return false;
4057+
4058+
Phi = SI;
4059+
if (!all_of(Phi->incoming_values(),
4060+
[](Value *V) { return isa<ConstantInt>(V); }))
4061+
return false;
4062+
continue;
4063+
}
40364064

4037-
PHINode *PHI = cast<PHINode>(GEPI.getPointerOperand());
4038-
if (GEPI.getParent() != PHI->getParent() ||
4039-
llvm::any_of(PHI->incoming_values(), [](Value *In) {
4040-
Instruction *I = dyn_cast<Instruction>(In);
4041-
return !I || isa<GetElementPtrInst>(I) || isa<PHINode>(I) ||
4042-
succ_empty(I->getParent()) ||
4043-
!I->getParent()->isLegalToHoistInto();
4044-
}))
4065+
if (!isa<ConstantInt>(Op))
4066+
return false;
4067+
}
4068+
4069+
if (!Phi)
40454070
return false;
40464071

40474072
LLVM_DEBUG(dbgs() << " Rewriting gep(phi) -> phi(gep):\n";
4048-
dbgs() << " original: " << *PHI << "\n";
4073+
dbgs() << " original: " << *Phi << "\n";
40494074
dbgs() << " " << GEPI << "\n";);
40504075

4051-
SmallVector<Value *, 4> Index(GEPI.indices());
4076+
auto GetNewOps = [&](Value *PhiOp) {
4077+
SmallVector<Value *> NewOps;
4078+
for (Value *Op : GEPI.operands())
4079+
if (Op == Phi)
4080+
NewOps.push_back(PhiOp);
4081+
else
4082+
NewOps.push_back(Op);
4083+
return NewOps;
4084+
};
4085+
4086+
IRB.SetInsertPoint(Phi);
4087+
PHINode *NewPhi = IRB.CreatePHI(GEPI.getType(), Phi->getNumIncomingValues(),
4088+
Phi->getName() + ".sroa.phi");
4089+
40524090
bool IsInBounds = GEPI.isInBounds();
4053-
IRB.SetInsertPoint(GEPI.getParent(), GEPI.getParent()->getFirstNonPHIIt());
4054-
PHINode *NewPN = IRB.CreatePHI(GEPI.getType(), PHI->getNumIncomingValues(),
4055-
PHI->getName() + ".sroa.phi");
4056-
for (unsigned I = 0, E = PHI->getNumIncomingValues(); I != E; ++I) {
4057-
BasicBlock *B = PHI->getIncomingBlock(I);
4058-
Value *NewVal = nullptr;
4059-
int Idx = NewPN->getBasicBlockIndex(B);
4060-
if (Idx >= 0) {
4061-
NewVal = NewPN->getIncomingValue(Idx);
4091+
Type *SourceTy = GEPI.getSourceElementType();
4092+
// We only handle arguments, constants, and static allocas here, so we can
4093+
// insert GEPs at the end of the entry block.
4094+
IRB.SetInsertPoint(GEPI.getFunction()->getEntryBlock().getTerminator());
4095+
for (unsigned I = 0, E = Phi->getNumIncomingValues(); I != E; ++I) {
4096+
Value *Op = Phi->getIncomingValue(I);
4097+
BasicBlock *BB = Phi->getIncomingBlock(I);
4098+
Value *NewGEP;
4099+
if (int NI = NewPhi->getBasicBlockIndex(BB); NI >= 0) {
4100+
NewGEP = NewPhi->getIncomingValue(NI);
40624101
} else {
4063-
Instruction *In = cast<Instruction>(PHI->getIncomingValue(I));
4064-
4065-
IRB.SetInsertPoint(In->getParent(), std::next(In->getIterator()));
4066-
Type *Ty = GEPI.getSourceElementType();
4067-
NewVal = IRB.CreateGEP(Ty, In, Index, In->getName() + ".sroa.gep",
4068-
IsInBounds);
4102+
SmallVector<Value *> NewOps = GetNewOps(Op);
4103+
NewGEP =
4104+
IRB.CreateGEP(SourceTy, NewOps[0], ArrayRef(NewOps).drop_front(),
4105+
Phi->getName() + ".sroa.gep", IsInBounds);
40694106
}
4070-
NewPN->addIncoming(NewVal, B);
4107+
NewPhi->addIncoming(NewGEP, BB);
40714108
}
40724109

40734110
Visited.erase(&GEPI);
4074-
GEPI.replaceAllUsesWith(NewPN);
4111+
GEPI.replaceAllUsesWith(NewPhi);
40754112
GEPI.eraseFromParent();
4076-
Visited.insert(NewPN);
4077-
enqueueUsers(*NewPN);
4113+
Visited.insert(NewPhi);
4114+
enqueueUsers(*NewPhi);
40784115

40794116
LLVM_DEBUG(dbgs() << " to: ";
40804117
for (Value *In
4081-
: NewPN->incoming_values()) dbgs()
4118+
: NewPhi->incoming_values()) dbgs()
40824119
<< "\n " << *In;
4083-
dbgs() << "\n " << *NewPN << '\n');
4120+
dbgs() << "\n " << *NewPhi << '\n');
40844121

40854122
return true;
40864123
}
40874124

40884125
bool visitGetElementPtrInst(GetElementPtrInst &GEPI) {
4089-
if (foldGEPSelect(GEPI))
4126+
if (unfoldGEPSelect(GEPI))
40904127
return true;
40914128

4092-
if (isa<PHINode>(GEPI.getPointerOperand()) && foldGEPPhi(GEPI))
4129+
if (unfoldGEPPhi(GEPI))
40934130
return true;
40944131

40954132
enqueueUsers(GEPI);

‎llvm/test/Transforms/SROA/phi-and-select.ll

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,13 @@ define i32 @test3(i32 %x) {
114114
; CHECK-LABEL: @test3(
115115
; CHECK-NEXT: entry:
116116
; CHECK-NEXT: switch i32 [[X:%.*]], label [[BB0:%.*]] [
117-
; CHECK-NEXT: i32 1, label [[BB1:%.*]]
118-
; CHECK-NEXT: i32 2, label [[BB2:%.*]]
119-
; CHECK-NEXT: i32 3, label [[BB3:%.*]]
120-
; CHECK-NEXT: i32 4, label [[BB4:%.*]]
121-
; CHECK-NEXT: i32 5, label [[BB5:%.*]]
122-
; CHECK-NEXT: i32 6, label [[BB6:%.*]]
123-
; CHECK-NEXT: i32 7, label [[BB7:%.*]]
117+
; CHECK-NEXT: i32 1, label [[BB1:%.*]]
118+
; CHECK-NEXT: i32 2, label [[BB2:%.*]]
119+
; CHECK-NEXT: i32 3, label [[BB3:%.*]]
120+
; CHECK-NEXT: i32 4, label [[BB4:%.*]]
121+
; CHECK-NEXT: i32 5, label [[BB5:%.*]]
122+
; CHECK-NEXT: i32 6, label [[BB6:%.*]]
123+
; CHECK-NEXT: i32 7, label [[BB7:%.*]]
124124
; CHECK-NEXT: ]
125125
; CHECK: bb0:
126126
; CHECK-NEXT: br label [[EXIT:%.*]]
@@ -733,6 +733,7 @@ define void @PR20822(i1 %c1, i1 %c2, ptr %ptr) {
733733
; CHECK-LABEL: @PR20822(
734734
; CHECK-NEXT: entry:
735735
; CHECK-NEXT: [[F_SROA_0:%.*]] = alloca i32, align 4
736+
; CHECK-NEXT: [[F1_SROA_GEP:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], ptr [[PTR:%.*]], i32 0, i32 0
736737
; CHECK-NEXT: br i1 [[C1:%.*]], label [[IF_END:%.*]], label [[FOR_COND:%.*]]
737738
; CHECK: for.cond:
738739
; CHECK-NEXT: br label [[IF_END]]
@@ -742,9 +743,8 @@ define void @PR20822(i1 %c1, i1 %c2, ptr %ptr) {
742743
; CHECK: if.then2:
743744
; CHECK-NEXT: br label [[IF_THEN5]]
744745
; CHECK: if.then5:
745-
; CHECK-NEXT: [[F1:%.*]] = phi ptr [ [[PTR:%.*]], [[IF_THEN2]] ], [ [[F_SROA_0]], [[IF_END]] ]
746-
; CHECK-NEXT: [[DOTFCA_0_GEP:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], ptr [[F1]], i32 0, i32 0
747-
; CHECK-NEXT: store i32 0, ptr [[DOTFCA_0_GEP]], align 4
746+
; CHECK-NEXT: [[F1_SROA_PHI:%.*]] = phi ptr [ [[F1_SROA_GEP]], [[IF_THEN2]] ], [ [[F_SROA_0]], [[IF_END]] ]
747+
; CHECK-NEXT: store i32 0, ptr [[F1_SROA_PHI]], align 4
748748
; CHECK-NEXT: ret void
749749
;
750750
entry:

0 commit comments

Comments
 (0)