[llvm] r294810 - [PM] Fix a bug in how I ported LoopDeletion to the new PM.

[Chandler Carruth via llvm-commits]

Author: chandlerc
Date: Fri Feb 10 18:09:30 2017
New Revision: 294810

URL: http://llvm.org/viewvc/llvm-project?rev=294810&view=rev
Log:
[PM] Fix a bug in how I ported LoopDeletion to the new PM.

This was marking the loop for deletion after the loop was deleted. This
almost works, except that when we do any kind of debug logging it starts
reading the name of the loop from deleted memory or otherwise blowing
up. This can fail in a bunch of ways. I recently added a test that
*always* does this, and it started failing on the sanitizer bots.

The fix is to mark the loop as deleted in the loop PM infrastructure
before we remove the loop. We can do this by passing the updater into
the routine. That also lets us simplify a bunch of other interface
components here for a net win.

Modified:
    llvm/trunk/lib/Transforms/Scalar/LoopDeletion.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/LoopDeletion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopDeletion.cpp?rev=294810&r1=294809&r2=294810&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopDeletion.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopDeletion.cpp Fri Feb 10 18:09:30 2017
@@ -100,16 +100,14 @@ static bool isLoopDead(Loop *L, ScalarEv
 /// This entire process relies pretty heavily on LoopSimplify form and LCSSA in
 /// order to make various safety checks work.
 ///
-/// \returns true if the loop is deleted.
-///
-/// This also sets the \p Changed output parameter to `true` if any changes
-/// were made. This may mutate the loop even if it is unable to delete it due
-/// to hoisting trivially loop invariant instructions out of the loop.
+/// \returns true if any changes were made. This may mutate the loop even if it
+/// is unable to delete it due to hoisting trivially loop invariant
+/// instructions out of the loop.
 ///
 /// This also updates the relevant analysis information in \p DT, \p SE, and \p
-/// LI.
+/// LI. It also updates the loop PM if an updater struct is provided.
 static bool deleteLoopIfDead(Loop *L, DominatorTree &DT, ScalarEvolution &SE,
-                             LoopInfo &LI, bool &Changed) {
+                             LoopInfo &LI, LPMUpdater *Updater = nullptr) {
   assert(L->isLCSSAForm(DT) && "Expected LCSSA!");
 
   // We can only remove the loop if there is a preheader that we can
@@ -139,14 +137,15 @@ static bool deleteLoopIfDead(Loop *L, Do
     return false;
 
   // Finally, we have to check that the loop really is dead.
+  bool Changed = false;
   if (!isLoopDead(L, SE, ExitingBlocks, ExitBlock, Changed, Preheader))
-    return false;
+    return Changed;
 
   // Don't remove loops for which we can't solve the trip count.
   // They could be infinite, in which case we'd be changing program behavior.
   const SCEV *S = SE.getMaxBackedgeTakenCount(L);
   if (isa<SCEVCouldNotCompute>(S))
-    return false;
+    return Changed;
 
   // Now that we know the removal is safe, remove the loop by changing the
   // branch from the preheader to go to the single exit block.
@@ -154,6 +153,10 @@ static bool deleteLoopIfDead(Loop *L, Do
   // Because we're deleting a large chunk of code at once, the sequence in which
   // we remove things is very important to avoid invalidation issues.
 
+  // If we have an LPM updater, tell it about the loop being removed.
+  if (Updater)
+    Updater->markLoopAsDeleted(*L);
+
   // Tell ScalarEvolution that the loop is deleted. Do this before
   // deleting the loop so that ScalarEvolution can look at the loop
   // to determine what it needs to clean up.
@@ -214,8 +217,6 @@ static bool deleteLoopIfDead(Loop *L, Do
 
   // The last step is to update LoopInfo now that we've eliminated this loop.
   LI.markAsRemoved(L);
-  Changed = true;
-
   ++NumDeleted;
 
   return true;
@@ -224,15 +225,8 @@ static bool deleteLoopIfDead(Loop *L, Do
 PreservedAnalyses LoopDeletionPass::run(Loop &L, LoopAnalysisManager &AM,
                                         LoopStandardAnalysisResults &AR,
                                         LPMUpdater &Updater) {
-  bool Changed = false;
-
-  if (deleteLoopIfDead(&L, AR.DT, AR.SE, AR.LI, Changed)) {
-    assert(Changed && "Cannot delete a loop without changing something!");
-    // Need to update the LPM about this loop going away.
-    Updater.markLoopAsDeleted(L);
-  } else if (!Changed) {
+  if (!deleteLoopIfDead(&L, AR.DT, AR.SE, AR.LI, &Updater))
     return PreservedAnalyses::all();
-  }
 
   return getLoopPassPreservedAnalyses();
 }
@@ -271,6 +265,5 @@ bool LoopDeletionLegacyPass::runOnLoop(L
   ScalarEvolution &SE = getAnalysis<ScalarEvolutionWrapperPass>().getSE();
   LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
 
-  bool Changed = false;
-  return deleteLoopIfDead(L, DT, SE, LI, Changed) || Changed;
+  return deleteLoopIfDead(L, DT, SE, LI);
 }