[PATCH] D82799: [IndVars] Delay forgetValue() call

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: mkazantsev, lebedev.ri, reames.
Herald added subscribers: llvm-commits, javed.absar, hiraditya.
Herald added a project: LLVM.

While working on D82730 <https://reviews.llvm.org/D82730> I ran into a case where nearly all compile-time is spent inside the forgetValue() call in rewriteLoopExitValues(). Currently it forgets values eagerly, regardless of whether the phi node will actually get replaced or not. This patch moves the forgetValue() call to just before the phi gets replaced, so we don't waste time forgetting values that don't get changed.

However, this causes a test change in elim-extend.ll. The reason is that an earlier Phi node replacement in IV widening did not perform the forgetValue() call, and as such left behind stale SCEVs (still correct, but possibly sub-accurate). Insert the missing call.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82799

Files:
  llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp


Index: llvm/lib/Transforms/Utils/LoopUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -1314,13 +1314,6 @@
       if (!SE->isSCEVable(PN->getType()))
         continue;
 
-      // It's necessary to tell ScalarEvolution about this explicitly so that
-      // it can walk the def-use list and forget all SCEVs, as it may not be
-      // watching the PHI itself. Once the new exit value is in place, there
-      // may not be a def-use connection between the loop and every instruction
-      // which got a SCEVAddRecExpr for that loop.
-      SE->forgetValue(PN);
-
       // Iterate over all of the values in all the PHI nodes.
       for (unsigned i = 0; i != NumPreds; ++i) {
         // If the value being merged in is not integer or is not defined
@@ -1441,6 +1434,13 @@
       continue;
     }
 
+    // It's necessary to tell ScalarEvolution about this explicitly so that
+    // it can walk the def-use list and forget all SCEVs, as it may not be
+    // watching the PHI itself. Once the new exit value is in place, there
+    // may not be a def-use connection between the loop and every instruction
+    // which got a SCEVAddRecExpr for that loop.
+    SE->forgetValue(Phi.PN);
+
     NumReplaced++;
     Instruction *Inst = cast<Instruction>(PN->getIncomingValue(Phi.Ith));
     PN->setIncomingValue(Phi.Ith, ExitVal);
Index: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -1234,6 +1234,7 @@
         WidePhi->addIncoming(DU.WideDef, UsePhi->getIncomingBlock(0));
         IRBuilder<> Builder(&*WidePhi->getParent()->getFirstInsertionPt());
         Value *Trunc = Builder.CreateTrunc(WidePhi, DU.NarrowDef->getType());
+        SE->forgetValue(UsePhi);
         UsePhi->replaceAllUsesWith(Trunc);
         DeadInsts.emplace_back(UsePhi);
         LLVM_DEBUG(dbgs() << "INDVARS: Widen lcssa phi " << *UsePhi << " to "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82799.274191.patch
Type: text/x-patch
Size: 2123 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200629/1894a851/attachment-0001.bin>