[PATCH] D109068: Fix a missing memoryssa update in breakLoopBackedge

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: nikic, fhahn, asbirlea.
Herald added subscribers: george.burgess.iv, bollu, hiraditya, mcrosier.
reames requested review of this revision.
Herald added a project: LLVM.

This is a case I'd missed in 6a8237.  The odd bit here is that missing the edge removal update seems to produce MemorySSA which verifies, but is still corrupt in a way which bothers following passes.  I wasn't able to reduce a single pass test case, which is why the reported test case is taken as is.

Posting for review to make sure there's nothing "interesting" about the use of the updater classes.  The invariants on these are a bit subtle, and I want a second opinion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109068

Files:
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/Transforms/LoopDeletion/bbi-59728.ll


Index: llvm/test/Transforms/LoopDeletion/bbi-59728.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LoopDeletion/bbi-59728.ll
@@ -0,0 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt %s -verify-memoryssa -passes='loop-mssa(loop-deletion,loop-simplifycfg)' -S | FileCheck %s
+
+; This is a case where we failed to update memory SSA correctly in
+; loop-deletion which escapes local verification, but causes a crash
+; in loop-simplifycfg.
+define void @func_45() {
+; CHECK-LABEL: @func_45(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_BODY45:%.*]]
+; CHECK:       for.body45:
+; CHECK-NEXT:    store i32 433429641, i32* undef, align 1
+; CHECK-NEXT:    br label [[FOR_BODY45]]
+;
+entry:
+  br label %for.body45
+
+for.body45:                                       ; preds = %for.end72, %entry
+  br label %for.body48
+
+for.body48:                                       ; preds = %for.body48, %for.body45
+  store i32 433429641, i32* undef, align 1
+  br i1 undef, label %for.body48, label %for.end72
+
+for.end72:                                        ; preds = %for.body48
+  br label %for.body45
+}
Index: llvm/lib/Transforms/Utils/LoopUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -729,8 +729,9 @@
       // and outer loop so the other target doesn't need to an exit
       if (L->isLoopExiting(Latch)) {
         // TODO: Generalize ConstantFoldTerminator so that it can be used
-        // here without invalidating LCSSA.  (Tricky case: header is an exit
-        // block of a preceeding sibling loop w/o dedicated exits.)
+        // here without invalidating LCSSA or MemorySSA.  (Tricky case for
+        // LCSSA: header is an exit block of a preceeding sibling loop w/o
+        // dedicated exits.)
         const unsigned ExitIdx = L->contains(BI->getSuccessor(0)) ? 1 : 0;
         BasicBlock *ExitBB = BI->getSuccessor(ExitIdx);
 
@@ -746,6 +747,8 @@
 
         BI->eraseFromParent();
         DTU.applyUpdates({{DominatorTree::Delete, Latch, Header}});
+        if (MSSA)
+          MSSAU->applyUpdates({{DominatorTree::Delete, Latch, Header}}, DT);
         return;
       }
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109068.369947.patch
Type: text/x-patch
Size: 2345 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210901/409196d4/attachment.bin>