Skip to content

Commit e47257e

Browse files
committed
Revert "Reland [SROA] Maintain shadow/backing alloca when some slices are noncapturnig read-only calls to allow alloca partitioning/promotion"
There seems to be one more uncaught problem, SROA may now end up trying to re-re-repromote the just-promoted shadow alloca, and do that endlessly. This reverts commit adc0984.
1 parent acf603b commit e47257e

File tree

2 files changed

+183
-273
lines changed

2 files changed

+183
-273
lines changed

llvm/lib/Transforms/Scalar/SROA.cpp

Lines changed: 3 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ class Slice {
167167
void makeUnsplittable() { UseAndIsSplittable.setInt(false); }
168168

169169
Use *getUse() const { return UseAndIsSplittable.getPointer(); }
170-
void setUse(Use *U) { UseAndIsSplittable.setPointer(U); }
171170

172171
bool isDead() const { return getUse() == nullptr; }
173172
void kill() { UseAndIsSplittable.setPointer(nullptr); }
@@ -219,7 +218,7 @@ class Slice {
219218
class llvm::sroa::AllocaSlices {
220219
public:
221220
/// Construct the slices of a particular alloca.
222-
AllocaSlices(const DataLayout &DL, AllocaInst &AI, bool &Changed);
221+
AllocaSlices(const DataLayout &DL, AllocaInst &AI);
223222

224223
/// Test whether a pointer to the allocation escapes our analysis.
225224
///
@@ -271,12 +270,6 @@ class llvm::sroa::AllocaSlices {
271270
return DeadUseIfPromotable;
272271
}
273272

274-
void forgetTheDead() {
275-
DeadUsers.clear();
276-
DeadUseIfPromotable.clear();
277-
DeadOperands.clear();
278-
};
279-
280273
/// Access the dead operands referring to this alloca.
281274
///
282275
/// These are operands which have cannot actually be used to refer to the
@@ -302,21 +295,11 @@ class llvm::sroa::AllocaSlices {
302295

303296
friend class AllocaSlices::SliceBuilder;
304297

305-
void formBackingAlloca(AllocaInst *AI, bool &Changed);
306-
307298
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
308299
/// Handle to alloca instruction to simplify method interfaces.
309300
AllocaInst &AI;
310301
#endif
311302

312-
/// Certain escaping uses of an alloca (non-capturing-ones)
313-
/// do not prevent promotion, but force retention of the alloca.
314-
/// This records if there are any such uses.
315-
bool NeedsBackingAlloca = false;
316-
317-
/// Track if there are any `select`s/PHI's involving the alloca pointers.
318-
bool HavePHINodesOrSelectInstrs = false;
319-
320303
/// The instruction responsible for this alloca not having a known set
321304
/// of slices.
322305
///
@@ -1072,35 +1055,18 @@ class AllocaSlices::SliceBuilder : public PtrUseVisitor<SliceBuilder> {
10721055
return;
10731056
}
10741057

1075-
AS.HavePHINodesOrSelectInstrs = true;
1076-
if (AS.NeedsBackingAlloca && AS.HavePHINodesOrSelectInstrs)
1077-
return PI.setAborted(&I);
1078-
10791058
insertUse(I, Offset, Size);
10801059
}
10811060

10821061
void visitPHINode(PHINode &PN) { visitPHINodeOrSelectInst(PN); }
10831062

10841063
void visitSelectInst(SelectInst &SI) { visitPHINodeOrSelectInst(SI); }
10851064

1086-
void visitCallBase(CallBase &CB) {
1087-
if (!IsOffsetKnown || !CB.doesNotCapture(U->getOperandNo()))
1088-
return PI.setAborted(&CB);
1089-
// If we know that the callee does not retain the pointer,
1090-
// then it does not prevent SROA, although we have to workaround this.
1091-
// However, for now, only allow uses, that, at most, read from said memory.
1092-
if (!CB.onlyReadsMemory() && !CB.onlyReadsMemory(U->getOperandNo()))
1093-
return PI.setAborted(&CB);
1094-
AS.NeedsBackingAlloca = true;
1095-
if (AS.NeedsBackingAlloca && AS.HavePHINodesOrSelectInstrs)
1096-
return PI.setAborted(&CB);
1097-
}
1098-
10991065
/// Disable SROA entirely if there are unhandled users of the alloca.
11001066
void visitInstruction(Instruction &I) { PI.setAborted(&I); }
11011067
};
11021068

1103-
AllocaSlices::AllocaSlices(const DataLayout &DL, AllocaInst &AI, bool &Changed)
1069+
AllocaSlices::AllocaSlices(const DataLayout &DL, AllocaInst &AI)
11041070
:
11051071
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
11061072
AI(AI),
@@ -1117,10 +1083,6 @@ AllocaSlices::AllocaSlices(const DataLayout &DL, AllocaInst &AI, bool &Changed)
11171083
return;
11181084
}
11191085

1120-
// We may have found that the pointer to the AI escapes, but isn't captured.
1121-
if (NeedsBackingAlloca)
1122-
formBackingAlloca(&AI, Changed);
1123-
11241086
llvm::erase_if(Slices, [](const Slice &S) { return S.isDead(); });
11251087

11261088
// Sort the uses. This arranges for the offsets to be in ascending order,
@@ -3625,122 +3587,6 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
36253587

36263588
} // end anonymous namespace
36273589

3628-
/// Apparently, we can promote the alloca, but some uses of the alloca
3629-
/// are calls (that don't capture it's address), which require for the
3630-
/// trace alloca to remain. To do so, we must form a new "backing" alloca,
3631-
/// which will be kept as an up-to-date backup of the to-be-promoted-alloca's
3632-
/// content, and used in it's place in these non-capturing calls.
3633-
/// FIXME: support non-readonly non-capturing calls.
3634-
void AllocaSlices::formBackingAlloca(AllocaInst *AllocaToPromote,
3635-
bool &Changed) {
3636-
assert(NeedsBackingAlloca &&
3637-
"Should not be called if there is no need to rewrite.");
3638-
3639-
// We are going to preserve all of the original instructions that were
3640-
// operating on the original alloca, so we must forget any instructions
3641-
// that were deemed as dead-to-be-deleted during normal promotion.
3642-
forgetTheDead();
3643-
3644-
Changed = true;
3645-
3646-
// Now, we want to retain all of the instructions operating on the original
3647-
// alloca, so to avoid much hassle, create a new alloca, and swap (RAUW) them.
3648-
AllocaInst *ShadowAlloca = cast<AllocaInst>(AllocaToPromote->clone());
3649-
ShadowAlloca->takeName(AllocaToPromote);
3650-
AllocaToPromote->setName(ShadowAlloca->getName() + ".prom");
3651-
ShadowAlloca->insertBefore(AllocaToPromote);
3652-
AllocaToPromote->replaceAllUsesWith(ShadowAlloca);
3653-
3654-
// Avoid recomputing the same pointer over and over again, cache it.
3655-
SmallDenseMap<std::pair<uint64_t, Type *>, Value *> RebasedPtrsCSE;
3656-
3657-
// Don't do anything fancy, just put new insts "right after" the alloca.
3658-
IRBuilderTy Builder(AllocaToPromote->getContext());
3659-
BasicBlock *AllocaToPromoteBB = AllocaToPromote->getParent();
3660-
Builder.SetInsertPoint(AllocaToPromoteBB,
3661-
AllocaToPromoteBB->getFirstInsertionPt());
3662-
3663-
// Give a pointer `Offset` bytes into the `AllocaToPromote` with `PtrTy` type.
3664-
auto getRebasedPtr = [&RebasedPtrsCSE, &Builder, AllocaToPromote,
3665-
DL = AllocaToPromote->getModule()->getDataLayout()](
3666-
PointerType *PtrTy, const uint64_t Offset) {
3667-
// Look it up in a cache first.
3668-
auto It = RebasedPtrsCSE.find({Offset, PtrTy});
3669-
if (It != RebasedPtrsCSE.end())
3670-
return It->second;
3671-
3672-
// Otherwise, create a new pointer, and cache it for the future.
3673-
Value *NewPtr = getAdjustedPtr(
3674-
Builder, DL, AllocaToPromote,
3675-
APInt(DL.getIndexSizeInBits(PtrTy->getAddressSpace()), Offset), PtrTy,
3676-
"");
3677-
RebasedPtrsCSE[{Offset, PtrTy}] = NewPtr;
3678-
3679-
return NewPtr;
3680-
};
3681-
3682-
// Some instructions may have several uses of an alloca, and there's
3683-
// a separate slice for each use, so we must cache each instruction
3684-
// we clone, so that we only clone it once,
3685-
// not for each slice that references it.
3686-
SmallDenseMap<Instruction *, Instruction *> InstrCloneMap;
3687-
3688-
// Now, let's just deal with each slice. Roughly, we need to clone each
3689-
// instruction that is referenced by a slice (once per instruction!),
3690-
// and change the appropriate pointer from pointing at the shadow alloca
3691-
// into pointing into the alloca we are going to promote.
3692-
//
3693-
// NOTE: the original instruction is generally preserved,
3694-
// because we need to maintain the content parity between the two allocas!
3695-
for (Slice &S : Slices) {
3696-
// Just completely ignore dead slices.
3697-
if (S.isDead())
3698-
continue;
3699-
3700-
// Which instruction does this slice represent?
3701-
Use *OrigUse = S.getUse();
3702-
auto *OrigInstr = cast<Instruction>(OrigUse->getUser());
3703-
3704-
// Now, we need to make a clone of this instruction, but operating on
3705-
// the alloca-to-be-promoted instead.
3706-
Instruction *ClonedInstr;
3707-
// Only clone instruction once! See if we already did that for this instr.
3708-
auto It = InstrCloneMap.find(OrigInstr);
3709-
if (It != InstrCloneMap.end())
3710-
ClonedInstr = It->second;
3711-
else {
3712-
// This is the first time this instruction is seen.
3713-
// Clone it next to the original instruction, and cache it.
3714-
ClonedInstr = OrigInstr->clone();
3715-
ClonedInstr->insertBefore(OrigInstr);
3716-
InstrCloneMap.insert({OrigInstr, ClonedInstr});
3717-
3718-
// Also, if the instruction was returning anything, we do that instead.
3719-
if (!ClonedInstr->getType()->isVoidTy()) {
3720-
assert(isa<LoadInst>(OrigInstr) &&
3721-
"Not expecting to encounter here anything other than a `load`.");
3722-
ClonedInstr->setName(OrigInstr->getName() + ".prom");
3723-
OrigInstr->replaceAllUsesWith(ClonedInstr);
3724-
}
3725-
3726-
if (isa<LoadInst>(OrigInstr))
3727-
// We know that all the offending (non-capturing) calls do not modify
3728-
// the content of the shadow alloca, so we do not need to propagate
3729-
// the content of the shadow alloca to the alloca-to-be-promoted.
3730-
DeadUsers.push_back(OrigInstr);
3731-
}
3732-
3733-
// Final touch: the slice should refer to the
3734-
// use of the alloca-to-be-promoted, while it currently refers to
3735-
// use of the shadow alloca, so rectify that.
3736-
Value *NewPtr = getRebasedPtr(cast<PointerType>(OrigUse->get()->getType()),
3737-
S.beginOffset());
3738-
Use &ClonedUse = ClonedInstr->getOperandUse(OrigUse->getOperandNo());
3739-
ClonedUse.set(NewPtr);
3740-
S.setUse(&ClonedUse);
3741-
}
3742-
}
3743-
37443590
/// Strip aggregate type wrapping.
37453591
///
37463592
/// This removes no-op aggregate types wrapping an underlying type. It will
@@ -4766,7 +4612,7 @@ bool SROAPass::runOnAlloca(AllocaInst &AI) {
47664612
Changed |= AggRewriter.rewrite(AI);
47674613

47684614
// Build the slices using a recursive instruction-visiting builder.
4769-
AllocaSlices AS(DL, AI, Changed);
4615+
AllocaSlices AS(DL, AI);
47704616
LLVM_DEBUG(AS.print(dbgs()));
47714617
if (AS.isEscaped())
47724618
return Changed;

0 commit comments

Comments
 (0)