[llvm] 3888464 - Temporarily revert "[SCEVExpander] Add helper to clean up instrs inserted while expanding."
Jordan Rupprecht via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 14 14:54:03 PDT 2020
Author: Jordan Rupprecht
Date: 2020-08-14T14:52:37-07:00
New Revision: 38884641f28e373ce291dc5ea93416756216e536
URL: https://github.com/llvm/llvm-project/commit/38884641f28e373ce291dc5ea93416756216e536
DIFF: https://github.com/llvm/llvm-project/commit/38884641f28e373ce291dc5ea93416756216e536.diff
LOG: Temporarily revert "[SCEVExpander] Add helper to clean up instrs inserted while expanding."
This reverts commit 7829c33084a7a5097533cf862daef521380c4e63. The assertion is triggering on some internal code. A reduced test case is in progress.
Added:
Modified:
llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
index 29012cb2bfdb..b6e6f0d780fe 100644
--- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
+++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
@@ -180,23 +180,6 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
ChainedPhis.clear();
}
- /// Return a vector containing all instructions inserted during expansion.
- SmallVector<Instruction *, 32> getAllInsertedInstructions() const {
- SmallVector<Instruction *, 32> Result;
- for (auto &VH : InsertedValues) {
- Value *V = VH;
- if (auto *Inst = dyn_cast<Instruction>(V))
- Result.push_back(Inst);
- }
- for (auto &VH : InsertedPostIncValues) {
- Value *V = VH;
- if (auto *Inst = dyn_cast<Instruction>(V))
- Result.push_back(Inst);
- }
-
- return Result;
- }
-
/// Return true for expressions that can't be evaluated at runtime
/// within given \b Budget.
///
@@ -469,27 +452,6 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
/// If no PHIs have been created, return the unchanged operand \p OpIdx.
Value *fixupLCSSAFormFor(Instruction *User, unsigned OpIdx);
};
-
-/// Helper to remove instructions inserted during SCEV expansion, unless they
-/// are marked as used.
-class SCEVExpanderCleaner {
- SCEVExpander &Expander;
-
- DominatorTree &DT;
-
- /// Indicates whether the result of the expansion is used. If false, the
- /// instructions added during expansion are removed.
- bool ResultUsed;
-
-public:
- SCEVExpanderCleaner(SCEVExpander &Expander, DominatorTree &DT)
- : Expander(Expander), DT(DT), ResultUsed(false) {}
-
- ~SCEVExpanderCleaner();
-
- /// Indicate that the result of the expansion is used.
- void markResultUsed() { ResultUsed = true; }
-};
} // namespace llvm
#endif
diff --git a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
index d0b6244efcce..52993c9dcb09 100644
--- a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
@@ -295,6 +295,31 @@ static void deleteDeadInstruction(Instruction *I) {
I->eraseFromParent();
}
+namespace {
+class ExpandedValuesCleaner {
+ SCEVExpander &Expander;
+ TargetLibraryInfo *TLI;
+ SmallVector<Value *, 4> ExpandedValues;
+ bool Commit = false;
+
+public:
+ ExpandedValuesCleaner(SCEVExpander &Expander, TargetLibraryInfo *TLI)
+ : Expander(Expander), TLI(TLI) {}
+
+ void add(Value *V) { ExpandedValues.push_back(V); }
+
+ void commit() { Commit = true; }
+
+ ~ExpandedValuesCleaner() {
+ if (!Commit) {
+ Expander.clear();
+ for (auto *V : ExpandedValues)
+ RecursivelyDeleteTriviallyDeadInstructions(V, TLI);
+ }
+ }
+};
+} // namespace
+
//===----------------------------------------------------------------------===//
//
// Implementation of LoopIdiomRecognize
@@ -908,7 +933,7 @@ bool LoopIdiomRecognize::processLoopStridedStore(
BasicBlock *Preheader = CurLoop->getLoopPreheader();
IRBuilder<> Builder(Preheader->getTerminator());
SCEVExpander Expander(*SE, *DL, "loop-idiom");
- SCEVExpanderCleaner ExpCleaner(Expander, *DT);
+ ExpandedValuesCleaner EVC(Expander, TLI);
Type *DestInt8PtrTy = Builder.getInt8PtrTy(DestAS);
Type *IntIdxTy = DL->getIndexType(DestPtr->getType());
@@ -931,6 +956,7 @@ bool LoopIdiomRecognize::processLoopStridedStore(
// base pointer and checking the region.
Value *BasePtr =
Expander.expandCodeFor(Start, DestInt8PtrTy, Preheader->getTerminator());
+ EVC.add(BasePtr);
// From here on out, conservatively report to the pass manager that we've
// changed the IR, even if we later clean up these added instructions. There
@@ -1015,7 +1041,7 @@ bool LoopIdiomRecognize::processLoopStridedStore(
if (MSSAU && VerifyMemorySSA)
MSSAU->getMemorySSA()->verifyMemorySSA();
++NumMemSet;
- ExpCleaner.markResultUsed();
+ EVC.commit();
return true;
}
@@ -1049,7 +1075,7 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(StoreInst *SI,
IRBuilder<> Builder(Preheader->getTerminator());
SCEVExpander Expander(*SE, *DL, "loop-idiom");
- SCEVExpanderCleaner ExpCleaner(Expander, *DT);
+ ExpandedValuesCleaner EVC(Expander, TLI);
bool Changed = false;
const SCEV *StrStart = StoreEv->getStart();
@@ -1068,6 +1094,7 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(StoreInst *SI,
// checking everything.
Value *StoreBasePtr = Expander.expandCodeFor(
StrStart, Builder.getInt8PtrTy(StrAS), Preheader->getTerminator());
+ EVC.add(StoreBasePtr);
// From here on out, conservatively report to the pass manager that we've
// changed the IR, even if we later clean up these added instructions. There
@@ -1095,6 +1122,7 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(StoreInst *SI,
// mutated by the loop.
Value *LoadBasePtr = Expander.expandCodeFor(
LdStart, Builder.getInt8PtrTy(LdAS), Preheader->getTerminator());
+ EVC.add(LoadBasePtr);
if (mayLoopAccessLocation(LoadBasePtr, ModRefInfo::Mod, CurLoop, BECount,
StoreSize, *AA, Stores))
@@ -1110,6 +1138,7 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(StoreInst *SI,
Value *NumBytes =
Expander.expandCodeFor(NumBytesS, IntIdxTy, Preheader->getTerminator());
+ EVC.add(NumBytes);
CallInst *NewCall = nullptr;
// Check whether to generate an unordered atomic memcpy:
@@ -1169,7 +1198,7 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(StoreInst *SI,
if (MSSAU && VerifyMemorySSA)
MSSAU->getMemorySSA()->verifyMemorySSA();
++NumMemCpy;
- ExpCleaner.markResultUsed();
+ EVC.commit();
return true;
}
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index bc2b11ce4f94..da6a719dccd1 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -1882,12 +1882,10 @@ Value *SCEVExpander::expand(const SCEV *S) {
// there) so that it is guaranteed to dominate any user inside the loop.
if (L && SE.hasComputableLoopEvolution(S, L) && !PostIncLoops.count(L))
InsertPt = &*L->getHeader()->getFirstInsertionPt();
-
while (InsertPt->getIterator() != Builder.GetInsertPoint() &&
(isInsertedInstruction(InsertPt) ||
- isa<DbgInfoIntrinsic>(InsertPt))) {
+ isa<DbgInfoIntrinsic>(InsertPt)))
InsertPt = &*std::next(InsertPt->getIterator());
- }
break;
}
}
@@ -2632,40 +2630,4 @@ bool isSafeToExpandAt(const SCEV *S, const Instruction *InsertionPoint,
}
return false;
}
-
-SCEVExpanderCleaner::~SCEVExpanderCleaner() {
- // Result is used, nothing to remove.
- if (ResultUsed)
- return;
-
- auto InsertedInstructions = Expander.getAllInsertedInstructions();
-#ifndef NDEBUG
- SmallPtrSet<Instruction *, 8> InsertedSet(InsertedInstructions.begin(),
- InsertedInstructions.end());
- (void)InsertedSet;
-#endif
- // Remove sets with value handles.
- Expander.clear();
-
- // Sort so that earlier instructions do not dominate later instructions.
- stable_sort(InsertedInstructions, [this](Instruction *A, Instruction *B) {
- return DT.dominates(B, A);
- });
- // Remove all inserted instructions.
- for (Instruction *I : InsertedInstructions) {
-
-#ifndef NDEBUG
- assert(all_of(I->users(),
- [&InsertedSet](Value *U) {
- return InsertedSet.contains(cast<Instruction>(U));
- }) &&
- "removed instruction should only be used by instructions inserted "
- "during expansion");
-#endif
- assert(!I->getType()->isVoidTy() &&
- "inserted instruction should have non-void types");
- I->replaceAllUsesWith(UndefValue::get(I->getType()));
- I->eraseFromParent();
- }
-}
}
More information about the llvm-commits
mailing list