[PATCH] D68194: [LCSSA] Forget values we create LCSSA phis for

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: efriedma, sanjoy.google, reames.
Herald added subscribers: javed.absar, hiraditya.
Herald added a project: LLVM.

Currently we only forget the loop we added LCSSA phis for. But SCEV
expressions in other loops could also depend on the instruction we added
a PHI for and currently we do not invalidate those expressions. This can
happen when we use ScalarEvolution before converting a function to LCSSA
form. The SCEV expressions will refer to the non-LCSSA value. If this
SCEV expression is then used with the expander, we do not preserve LCSSA
form.

This patch properly forgets the values we created PHIs for. Those need
to be recomputed again. This patch fixes PR43458.

Currently SCEV::verify does not catch this mismatch and any test would
need to run multiple passes to trigger the error (e.g. -loop-reduce
-loop-unroll). I will also look into catching this kind of mismatch in
the verifier. Also, we currently forget the whole loop in LCSSA and I'll
check if we can be more surgical.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68194

Files:
  llvm/include/llvm/Transforms/Utils/LoopUtils.h
  llvm/lib/Transforms/Utils/LCSSA.cpp


Index: llvm/lib/Transforms/Utils/LCSSA.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LCSSA.cpp
+++ llvm/lib/Transforms/Utils/LCSSA.cpp
@@ -74,7 +74,8 @@
 /// that are outside the current loop.  If so, insert LCSSA PHI nodes and
 /// rewrite the uses.
 bool llvm::formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
-                                    DominatorTree &DT, LoopInfo &LI) {
+                                    DominatorTree &DT, LoopInfo &LI,
+                                    ScalarEvolution *SE) {
   SmallVector<Use *, 16> UsesToRewrite;
   SmallSetVector<PHINode *, 16> PHIsToRemove;
   PredIteratorCache PredCache;
@@ -134,6 +135,11 @@
     SSAUpdater SSAUpdate(&InsertedPHIs);
     SSAUpdate.Initialize(I->getType(), I->getName());
 
+    // Force re-computation of I, as some users now need to use the new PHI
+    // node.
+    if (SE)
+      SE->forgetValue(I);
+
     // Insert the LCSSA phi's into all of the exit blocks dominated by the
     // value, and add them to the Phi's map.
     for (BasicBlock *ExitBB : ExitBlocks) {
@@ -368,7 +374,7 @@
       Worklist.push_back(&I);
     }
   }
-  Changed = formLCSSAForInstructions(Worklist, DT, *LI);
+  Changed = formLCSSAForInstructions(Worklist, DT, *LI, SE);
 
   // If we modified the code, remove any caches about the loop from SCEV to
   // avoid dangling entries.
Index: llvm/include/llvm/Transforms/Utils/LoopUtils.h
===================================================================
--- llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -73,7 +73,8 @@
 ///
 /// Returns true if any modifications are made.
 bool formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
-                              DominatorTree &DT, LoopInfo &LI);
+                              DominatorTree &DT, LoopInfo &LI,
+                              ScalarEvolution *SE);
 
 /// Put loop into LCSSA form.
 ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68194.222329.patch
Type: text/x-patch
Size: 2009 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190929/b515d557/attachment.bin>