[PATCH] D50888: [NFC][LICM] Remove too conservative variable

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: reames, sanjoy, skatkov, fedor.sergeev.

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.


https://reviews.llvm.org/D50888

Files:
  lib/Transforms/Scalar/LICM.cpp


Index: lib/Transforms/Scalar/LICM.cpp
===================================================================
--- lib/Transforms/Scalar/LICM.cpp
+++ lib/Transforms/Scalar/LICM.cpp
@@ -458,14 +458,10 @@
     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;) {
@@ -493,10 +489,9 @@
       //
       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;
@@ -527,15 +522,13 @@
 
       using namespace PatternMatch;
       if (match(&I, m_Intrinsic<Intrinsic::experimental_guard>()) &&
-          IsMustExecute && IsMemoryNotModified &&
-          CurLoop->hasLoopInvariantOperands(&I)) {
+          IsMemoryNotModified && CurLoop->hasLoopInvariantOperands(&I) &&
+          SafetyInfo->mustExecuteInLoop(CurLoop, &I, DT)) {
         hoist(I, DT, CurLoop, SafetyInfo, ORE);
         Changed = true;
         continue;
       }
 
-      if (IsMustExecute)
-        IsMustExecute = isGuaranteedToTransferExecutionToSuccessor(&I);
       if (IsMemoryNotModified)
         IsMemoryNotModified = !I.mayWriteToMemory();
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50888.161194.patch
Type: text/x-patch
Size: 2354 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180817/6e9d3393/attachment.bin>