[llvm-branch-commits] [LPM][LegacyPM] Reenable LCSSA Verification (PR #191667)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Aiden Grossman (boomanaiden154)

<details>
<summary>Changes</summary>

This was disabled about a decade ago due to issues with LoopSink.
LoopSink has since had its LegacyPM version removed and is now a
function pass due to not needing too much loop infrastructure. So we can
try enabling this again to prevent backsliding on important cases while
we work on switching to the NewPM which does enforce these things.

Eventually we will want to add assertions here for LoopStrengthReduce,
but given it does not correctly preserve LCSSA, postpone that for now.


---
Full diff: https://github.com/llvm/llvm-project/pull/191667.diff


1 Files Affected:

- (modified) llvm/lib/Analysis/LoopPass.cpp (+3-6) 


``````````diff
diff --git a/llvm/lib/Analysis/LoopPass.cpp b/llvm/lib/Analysis/LoopPass.cpp
index d8680aac74b22..3d8621c0c4fa2 100644
--- a/llvm/lib/Analysis/LoopPass.cpp
+++ b/llvm/lib/Analysis/LoopPass.cpp
@@ -130,8 +130,9 @@ bool LPPassManager::runOnFunction(Function &F) {
   auto &LIWP = getAnalysis<LoopInfoWrapperPass>();
   LI = &LIWP.getLoopInfo();
   Module &M = *F.getParent();
-#if 0
-  DominatorTree *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
+#ifndef NDEBUG
+  DominatorTree *DT =
+      &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
 #endif
   bool Changed = false;
 
@@ -239,12 +240,8 @@ bool LPPassManager::runOnFunction(Function &F) {
         // is that LPPassManager might run passes which do not require LCSSA
         // form (LoopPassPrinter for example). We should skip verification for
         // such passes.
-        // FIXME: Loop-sink currently break LCSSA. Fix it and reenable the
-        // verification!
-#if 0
         if (mustPreserveAnalysisID(LCSSAVerificationPass::ID))
           assert(CurrentLoop->isRecursivelyLCSSAForm(*DT, *LI));
-#endif
 
         // Then call the regular verifyAnalysis functions.
         verifyPreservedAnalysis(P);

``````````

</details>


https://github.com/llvm/llvm-project/pull/191667