[PATCH] D81236: Correctly report modified status for LoopExtractor

[serge via Phabricator via llvm-commits]

serge-sans-paille updated this revision to Diff 271195.
serge-sans-paille added a comment.

Try another approach and fix the fact that the return status of a lazy transformation is simply ignored.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81236/new/

https://reviews.llvm.org/D81236

Files:
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Transforms/IPO/LoopExtractor.cpp


Index: llvm/lib/Transforms/IPO/LoopExtractor.cpp
===================================================================
--- llvm/lib/Transforms/IPO/LoopExtractor.cpp
+++ llvm/lib/Transforms/IPO/LoopExtractor.cpp
@@ -18,6 +18,7 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
@@ -55,11 +56,9 @@
     bool extractLoop(Loop *L, LoopInfo &LI, DominatorTree &DT);
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
-      AU.addRequiredID(BreakCriticalEdgesID);
       AU.addRequired<DominatorTreeWrapperPass>();
       AU.addRequired<LoopInfoWrapperPass>();
       AU.addPreserved<LoopInfoWrapperPass>();
-      AU.addRequiredID(LoopSimplifyID);
       AU.addUsedIfAvailable<AssumptionCacheTracker>();
     }
   };
@@ -68,10 +67,8 @@
 char LoopExtractor::ID = 0;
 INITIALIZE_PASS_BEGIN(LoopExtractor, "loop-extract",
                       "Extract loops into new functions", false, false)
-INITIALIZE_PASS_DEPENDENCY(BreakCriticalEdges)
 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(LoopSimplify)
 INITIALIZE_PASS_END(LoopExtractor, "loop-extract",
                     "Extract loops into new functions", false, false)
 
@@ -131,18 +128,23 @@
   if (F.empty())
     return false;
 
+  legacy::FunctionPassManager FPM(F.getParent());
+  FPM.add(createBreakCriticalEdgesPass());
+  FPM.add(createLoopSimplifyPass());
+  bool Changed = FPM.run(F);
+
   LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>(F).getLoopInfo();
 
   // If there are no loops in the function.
   if (LI.empty())
-    return false;
+    return Changed;
 
   DominatorTree &DT = getAnalysis<DominatorTreeWrapperPass>(F).getDomTree();
 
   // If there is more than one top-level loop in this function, extract all of
   // the loops.
   if (std::next(LI.begin()) != LI.end())
-    return extractLoops(LI.begin(), LI.end(), LI, DT);
+    return Changed | extractLoops(LI.begin(), LI.end(), LI, DT);
 
   // Otherwise there is exactly one top-level loop.
   Loop *TLL = *LI.begin();
@@ -171,14 +173,14 @@
     }
 
     if (ShouldExtractLoop)
-      return extractLoop(TLL, LI, DT);
+      return Changed | extractLoop(TLL, LI, DT);
   }
 
   // Okay, this function is a minimal container around the specified loop.
   // If we extract the loop, we will continue to just keep extracting it
   // infinitely... so don't extract it. However, if the loop contains any
   // sub-loops, extract them.
-  return extractLoops(TLL->begin(), TLL->end(), LI, DT);
+  return Changed | extractLoops(TLL->begin(), TLL->end(), LI, DT);
 }
 
 bool LoopExtractor::extractLoops(Loop::iterator From, Loop::iterator To,
Index: llvm/lib/IR/LegacyPassManager.cpp
===================================================================
--- llvm/lib/IR/LegacyPassManager.cpp
+++ llvm/lib/IR/LegacyPassManager.cpp
@@ -1670,7 +1670,8 @@
   assert(FPP && "Unable to find on the fly pass");
 
   FPP->releaseMemoryOnTheFly();
-  FPP->run(F);
+  bool Changed = FPP->run(F);
+  assert(!Changed && "An analysis pass shouldn't modify its content.");
   return ((PMTopLevelManager*)FPP)->findAnalysisPass(PI);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81236.271195.patch
Type: text/x-patch
Size: 3326 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200616/2da23af5/attachment.bin>