-
Notifications
You must be signed in to change notification settings - Fork 15.4k
AMDGPU/PromoteAlloca: Simplify how deferred loads work #170510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit-id:c5c3b1e8
commit-id:97c65f02
The second pass of promotion to vector can be quite simple. Reflect that simplicity in the code for better maintainability. commit-id:cbc1e9ae
|
@llvm/pr-subscribers-backend-amdgpu Author: Nicolai Hähnle (nhaehnle) ChangesThe second pass of promotion to vector can be quite simple. Reflect that Stack:
Full diff: https://github.com/llvm/llvm-project/pull/170510.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
index 77db14513254f..73ec607014d31 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -502,27 +502,14 @@ static Value *promoteAllocaUserToVector(
Instruction *Inst, const DataLayout &DL, FixedVectorType *VectorTy,
unsigned VecStoreSize, unsigned ElementSize,
DenseMap<MemTransferInst *, MemTransferInfo> &TransferInfo,
- std::map<GetElementPtrInst *, WeakTrackingVH> &GEPVectorIdx, Value *CurVal,
- SmallVectorImpl<LoadInst *> &DeferredLoads) {
+ std::map<GetElementPtrInst *, WeakTrackingVH> &GEPVectorIdx,
+ function_ref<Value *()> GetCurVal) {
// Note: we use InstSimplifyFolder because it can leverage the DataLayout
// to do more folding, especially in the case of vector splats.
IRBuilder<InstSimplifyFolder> Builder(Inst->getContext(),
InstSimplifyFolder(DL));
Builder.SetInsertPoint(Inst);
- const auto GetOrLoadCurrentVectorValue = [&]() -> Value * {
- if (CurVal)
- return CurVal;
-
- // If the current value is not known, insert a dummy load and lower it on
- // the second pass.
- LoadInst *Dummy =
- Builder.CreateLoad(VectorTy, PoisonValue::get(Builder.getPtrTy()),
- "promotealloca.dummyload");
- DeferredLoads.push_back(Dummy);
- return Dummy;
- };
-
const auto CreateTempPtrIntCast = [&Builder, DL](Value *Val,
Type *PtrTy) -> Value * {
assert(DL.getTypeStoreSize(Val->getType()) == DL.getTypeStoreSize(PtrTy));
@@ -542,12 +529,7 @@ static Value *promoteAllocaUserToVector(
switch (Inst->getOpcode()) {
case Instruction::Load: {
- // Loads can only be lowered if the value is known.
- if (!CurVal) {
- DeferredLoads.push_back(cast<LoadInst>(Inst));
- return nullptr;
- }
-
+ Value *CurVal = GetCurVal();
Value *Index = calculateVectorIndex(
cast<LoadInst>(Inst)->getPointerOperand(), GEPVectorIdx);
@@ -637,7 +619,7 @@ static Value *promoteAllocaUserToVector(
Val = Builder.CreateBitOrPointerCast(Val, SubVecTy);
- Value *CurVec = GetOrLoadCurrentVectorValue();
+ Value *CurVec = GetCurVal();
for (unsigned K = 0, NumElts = std::min(NumWrittenElts, NumVecElts);
K < NumElts; ++K) {
Value *CurIdx =
@@ -650,8 +632,7 @@ static Value *promoteAllocaUserToVector(
if (Val->getType() != VecEltTy)
Val = Builder.CreateBitOrPointerCast(Val, VecEltTy);
- return Builder.CreateInsertElement(GetOrLoadCurrentVectorValue(), Val,
- Index);
+ return Builder.CreateInsertElement(GetCurVal(), Val, Index);
}
case Instruction::Call: {
if (auto *MTI = dyn_cast<MemTransferInst>(Inst)) {
@@ -673,7 +654,7 @@ static Value *promoteAllocaUserToVector(
}
}
- return Builder.CreateShuffleVector(GetOrLoadCurrentVectorValue(), Mask);
+ return Builder.CreateShuffleVector(GetCurVal(), Mask);
}
if (auto *MSI = dyn_cast<MemSetInst>(Inst)) {
@@ -1038,37 +1019,44 @@ bool AMDGPUPromoteAllocaImpl::tryPromoteAllocaToVector(AllocaInst &Alloca) {
Updater.AddAvailableValue(EntryBB, AllocaInitValue);
- // First handle the initial worklist.
- SmallVector<LoadInst *, 4> DeferredLoads;
+ // First handle the initial worklist, in basic block order.
+ //
+ // Insert a placeholder whenever we need the vector value at the top of a
+ // basic block.
+ SmallVector<Instruction *> Placeholders;
forEachWorkListItem(WorkList, [&](Instruction *I) {
BasicBlock *BB = I->getParent();
- // On the first pass, we only take values that are trivially known, i.e.
- // where AddAvailableValue was already called in this block.
- Value *Result = promoteAllocaUserToVector(
- I, *DL, VectorTy, VecStoreSize, ElementSize, TransferInfo, GEPVectorIdx,
- Updater.FindValueForBlock(BB), DeferredLoads);
+ auto GetCurVal = [&]() -> Value * {
+ if (Value *CurVal = Updater.FindValueForBlock(BB))
+ return CurVal;
+
+ // If the current value in the basic block is not yet known, insert a
+ // placeholder that we will replace later.
+ IRBuilder<> Builder(I);
+ auto *Placeholder = cast<Instruction>(Builder.CreateFreeze(
+ PoisonValue::get(VectorTy), "promotealloca.placeholder"));
+ Placeholders.push_back(Placeholder);
+ Updater.AddAvailableValue(BB, Placeholder);
+ return Placeholder;
+ };
+
+ Value *Result =
+ promoteAllocaUserToVector(I, *DL, VectorTy, VecStoreSize, ElementSize,
+ TransferInfo, GEPVectorIdx, GetCurVal);
if (Result)
Updater.AddAvailableValue(BB, Result);
});
- // Then handle deferred loads.
- forEachWorkListItem(DeferredLoads, [&](Instruction *I) {
- SmallVector<LoadInst *, 0> NewDLs;
- BasicBlock *BB = I->getParent();
- // On the second pass, we use GetValueInMiddleOfBlock to guarantee we always
- // get a value, inserting PHIs as needed.
- Value *Result = promoteAllocaUserToVector(
- I, *DL, VectorTy, VecStoreSize, ElementSize, TransferInfo, GEPVectorIdx,
- Updater.GetValueInMiddleOfBlock(I->getParent()), NewDLs);
- if (Result)
- Updater.AddAvailableValue(BB, Result);
- assert(NewDLs.empty() && "No more deferred loads should be queued!");
- });
+ // Now fixup the placeholders.
+ for (Instruction *Placeholder : Placeholders) {
+ Placeholder->replaceAllUsesWith(
+ Updater.GetValueInMiddleOfBlock(Placeholder->getParent()));
+ Placeholder->eraseFromParent();
+ }
// Delete all instructions. On the first pass, new dummy loads may have been
// added so we need to collect them too.
DenseSet<Instruction *> InstsToDelete(WorkList.begin(), WorkList.end());
- InstsToDelete.insert_range(DeferredLoads);
for (Instruction *I : InstsToDelete) {
assert(I->use_empty());
I->eraseFromParent();
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…)" This reverts commit 22a2c27. Failure on clang-hip-vega20: https://lab.llvm.org/buildbot/#/builders/123/builds/31779
…)" (#170955) The second pass of promotion to vector can be quite simple. Reflect that simplicity in the code for better maintainability. v2: - don't put placeholders into the SSAUpdater, and add a test that shows the problem
The second pass of promotion to vector can be quite simple. Reflect that simplicity in the code for better maintainability.
…170510)" This reverts commit 22a2c27. Failure on clang-hip-vega20: https://lab.llvm.org/buildbot/#/builders/123/builds/31779
…170510)" (llvm#170955) The second pass of promotion to vector can be quite simple. Reflect that simplicity in the code for better maintainability. v2: - don't put placeholders into the SSAUpdater, and add a test that shows the problem
The second pass of promotion to vector can be quite simple. Reflect that
simplicity in the code for better maintainability.
Stack: