[NFC][LICM] Remove too conservative IsMustExecute variable

[mkazantsev]

Summary

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 instructions, the check in isSafeToExecuteUnconditionally is strictly more precise than this
variable, so we can safely give it up.

For the guards, it might be too conservative to use this variable, we can instead use a more precise
logic from MustExecute. 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.

[fedor.sergeev]

lib/Transforms/Scalar/LICM.cpp

464 ↗

(On Diff #161194)

Since you are going to remove isGuaranteedToExecute perhaps mention something else here?
Or just drop the reference.

[mkazantsev]

lib/Transforms/Scalar/LICM.cpp

464 ↗

(On Diff #161194)

Right, I will do it when I drop it.

[fedor.sergeev]

LGTM

[reames]

lib/Transforms/Scalar/LICM.cpp

523 ↗

(On Diff #161194)

We should be able to sink this inside the normal hoisting now since the must execute case is now handled. Suggestion as a follow on change, nothing more.

llvm/trunk/lib/Transforms/Scalar/LICM.cpp

bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI,
   bool Changed = false;
   for (DomTreeNode *DTN : Worklist) {
     BasicBlock *BB = DTN->getBlock();
     // Only need to process the contents of this block if it is not part of a
     // subloop (which would already have been processed).
     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;) {
       Instruction &I = *II++;
       // Try constant folding this instruction.  If all the operands are
       // constants, it is technically hoistable, but it would be better to
       // just fold it.
       if (Constant *C = ConstantFoldInstruction(
for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E;) {
       }
 
       // Try hoisting the instruction out to the preheader.  We can only do
       // this if all of the operands of the instruction are loop invariant and
       // if it is safe to hoist the instruction.
       //
       if (CurLoop->hasLoopInvariantOperands(&I) &&
           canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, true, ORE) &&
-          (IsMustExecute ||
-           isSafeToExecuteUnconditionally(
-               I, DT, CurLoop, SafetyInfo, ORE,
-               CurLoop->getLoopPreheader()->getTerminator()))) {
+              CurLoop->getLoopPreheader()->getTerminator())) {
         hoist(I, DT, CurLoop, SafetyInfo, ORE);
         Changed = true;
         continue;
       }
 
       // Attempt to remove floating point division out of the loop by
       // converting it to a reciprocal multiplication.
       if (I.getOpcode() == Instruction::FDiv &&
for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E;) {
         Changed = true;
         continue;
       }
 
       using namespace PatternMatch;
       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();
     }
   }
 
   return Changed;
 }