Skip to content

Commit

Permalink
[LICM] Remove too conservative IsMustExecute variable
Browse files Browse the repository at this point in the history
LICM relies on variable `MustExecute` which is conservatively set to `false`
in all non-headers. It is used when we decide whether or not we want to hoist
an instruction or a guard.

For the guards, it might be too conservative to use this variable, we can
instead use a more precise logic from LoopSafetyInfo. Currently it is only NFC
because `IsMemoryNotModified` is also conservatively set to `false` for all
non-headers, and we cannot hoist guards from non-header blocks. However once we
give up using `IsMemoryNotModified` and use a smarter check instead, this will
allow us to hoist guards from all mustexecute non-header blocks.

Differential Revision: https://reviews.llvm.org/D50888
Reveiwed By: fedor.sergeev

llvm-svn: 346204
  • Loading branch information
Max Kazantsev committed Nov 6, 2018
1 parent 96d1251 commit 0042c06
Showing 1 changed file with 8 additions and 15 deletions.
23 changes: 8 additions & 15 deletions llvm/lib/Transforms/Scalar/LICM.cpp
Expand Up @@ -460,14 +460,10 @@ bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI,
if (inSubLoop(BB, CurLoop, LI))
continue;

// Keep track of whether the prefix of instructions visited so far are such
// that the next instruction visited is guaranteed to execute if the loop
// is entered.
bool IsMustExecute = CurLoop->getHeader() == BB;
// Keep track of whether the prefix instructions could have written memory.
// TODO: This and IsMustExecute may be done smarter if we keep track of all
// throwing and mem-writing operations in every block, e.g. using something
// similar to isGuaranteedToExecute.
// TODO: This may be done smarter if we keep track of all throwing and
// mem-writing operations in every block, e.g. using something similar to
// isGuaranteedToExecute.
bool IsMemoryNotModified = CurLoop->getHeader() == BB;

for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E;) {
Expand All @@ -493,10 +489,9 @@ bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI,
//
if (CurLoop->hasLoopInvariantOperands(&I) &&
canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, true, ORE) &&
(IsMustExecute ||
isSafeToExecuteUnconditionally(
I, DT, CurLoop, SafetyInfo, ORE,
CurLoop->getLoopPreheader()->getTerminator()))) {
isSafeToExecuteUnconditionally(
I, DT, CurLoop, SafetyInfo, ORE,
CurLoop->getLoopPreheader()->getTerminator())) {
hoist(I, DT, CurLoop, SafetyInfo, ORE);
Changed = true;
continue;
Expand Down Expand Up @@ -531,15 +526,13 @@ bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI,
if (((I.use_empty() &&
match(&I, m_Intrinsic<Intrinsic::invariant_start>())) ||
isGuard(&I)) &&
IsMustExecute && IsMemoryNotModified &&
CurLoop->hasLoopInvariantOperands(&I)) {
IsMemoryNotModified && CurLoop->hasLoopInvariantOperands(&I) &&
SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop)) {
hoist(I, DT, CurLoop, SafetyInfo, ORE);
Changed = true;
continue;
}

if (IsMustExecute)
IsMustExecute = isGuaranteedToTransferExecutionToSuccessor(&I);
if (IsMemoryNotModified)
IsMemoryNotModified = !I.mayWriteToMemory();
}
Expand Down

0 comments on commit 0042c06

Please sign in to comment.