[PATCH] D96727: [NPM] Properly reset parent loop after loop passes

[Ta-Wei Tu via Phabricator via llvm-commits]

TaWeiTu created this revision.
TaWeiTu added reviewers: uabelho, fhahn, aeubanks.
Herald added a subscriber: hiraditya.
TaWeiTu requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This fixes https://bugs.llvm.org/show_bug.cgi?id=49185

When `NDEBUG` is not set, `LPMUpdater` checks if the added loops have the same parent loop as the current one in `addSiblingLoops`.
If multiple loop passes are executed through `LoopPassManager`, `U.ParentL` will be the same across all passes.
However, the parent loop might change after running a loop pass, resulting in assertion failures in subsequent passes.

This patch resets `U.ParentL` after running individual loop passes in `LoopPassManager`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96727

Files:
  llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp


Index: llvm/lib/Transforms/Scalar/LoopPassManager.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopPassManager.cpp
+++ llvm/lib/Transforms/Scalar/LoopPassManager.cpp
@@ -114,6 +114,13 @@
     // Check if the current pass preserved the loop-nest object or not.
     IsLoopNestPtrValid &= PassPA->getChecker<LoopNestAnalysis>().preserved();
 
+#ifndef NDEBUG
+    // After running the loop pass, the parent loop might change and we need to
+    // notify the updater, otherwise U.ParentL might gets outdated and triggers
+    // assertion failures in addSiblingLoops and addChildLoops.
+    U.setParentLoop(L.getParentLoop());
+#endif
+
     // FIXME: Historically, the pass managers all called the LLVM context's
     // yield function here. We don't have a generic way to acquire the
     // context and it isn't yet clear what the right pattern is for yielding
@@ -158,6 +165,13 @@
     // aggregate preserved set for this pass manager.
     PA.intersect(std::move(*PassPA));
 
+#ifndef NDEBUG
+    // After running the loop pass, the parent loop might change and we need to
+    // notify the updater, otherwise U.ParentL might gets outdated and triggers
+    // assertion failures in addSiblingLoops and addChildLoops.
+    U.setParentLoop(L.getParentLoop());
+#endif
+
     // FIXME: Historically, the pass managers all called the LLVM context's
     // yield function here. We don't have a generic way to acquire the
     // context and it isn't yet clear what the right pattern is for yielding
Index: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
===================================================================
--- llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
+++ llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
@@ -271,6 +271,10 @@
       SkipCurrentLoop = true;
   }
 
+#ifndef NDEBUG
+  void setParentLoop(Loop *L) { ParentL = L; }
+#endif
+
   /// Loop passes should use this method to indicate they have added new child
   /// loops of the current loop.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96727.323816.patch
Type: text/x-patch
Size: 2057 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210215/12dfba45/attachment.bin>