[PATCH] D87992: [MemorySSA] Fix removing dead accesses from MemoryPhis.

[Florian Hahn via Phabricator via llvm-commits]

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

The current logic to remove all uses of MemoryAccess defined in dead
blocks from MemoryPhis outside the set of dead blocks. MemoryPhis using
a definition from a dead block do not necessarily have to be in a direct
successor of a dead block, for example if the successor does not contain
any memory accesses.

To catch those additional MemoryPhis that need updating we need to check
all uses of all memory defs in the dead blocks and remove them from any
MemoryPhi outside the set of dead blocks.

Note that we sill need the existing code where we check the successors
for memory phis to catch the case where a MemoryPhi has an incoming
value from a dead block but is not defined in any dead block.

This can happen during unswitching, where a region between the memory
def and a memory phi becomes unreachable.

I *think* there's still a case we might miss in the above case: when the
memory phi is not a direct successor of a dead block. Not sure if this
can happen in practice.

Fixes PR47557.

Note that I did not manage to reduce the test cases so far that it only
requires a single pass to crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87992

Files:
  llvm/lib/Analysis/MemorySSAUpdater.cpp
  llvm/test/Analysis/MemorySSA/update-remove-dead-blocks.ll


Index: llvm/test/Analysis/MemorySSA/update-remove-dead-blocks.ll
===================================================================
--- /dev/null
+++ llvm/test/Analysis/MemorySSA/update-remove-dead-blocks.ll
@@ -0,0 +1,46 @@
+; RUN: opt -loop-unswitch -loop-reduce -loop-simplifycfg -verify-memoryssa -S %s | FileCheck %s
+
+; TODO: also run with NPM, but currently LSR does not preserve LCSSA, causing a verification failure on the test.
+;   opt -passes='loop-mssa(unswitch<nontrivial>,loop-reduce,simplify-cfg)' -verify-memoryssa -S %s | FileCheck %s
+
+; Test case for PR47557.
+
+; REQUIRES: x86-registered-target
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at a = external global i32, align 4
+ at c = external global [1 x i32], align 4
+
+define i32* @test() {
+; CHECK-LABEL: @test
+;
+entry:                                                ; preds = %entry
+  br label %for.cond
+
+for.cond:                                         ; preds = %cleanup, %entry
+  %storemerge = phi i64 [ 0, %entry ], [ %inc7, %cleanup ]
+  br label %for.cond2.1
+
+for.body3:                                        ; preds = %for.cond2.2, %for.cond2.1
+  %arrayidx = getelementptr inbounds [1 x i32], [1 x i32]* @c, i64 0, i64 %storemerge
+  ret i32* %arrayidx
+
+cleanup:                                          ; preds = %for.end5, %if.then
+  %inc7 = add nsw i64 %storemerge, 1
+  br label %for.cond
+
+for.cond2.1:                                      ; preds = %for.cond
+  br i1 true, label %for.inc.1, label %for.body3
+
+for.inc.1:                                        ; preds = %for.end.1
+  br i1 false, label %for.body.2, label %cleanup
+
+for.body.2:                                       ; preds = %for.inc.1
+  store i32 0, i32* @a, align 4
+  br label %for.cond2.2
+
+for.cond2.2:                                      ; preds = %for.body.2
+  br i1 true, label %cleanup, label %for.body3
+}
Index: llvm/lib/Analysis/MemorySSAUpdater.cpp
===================================================================
--- llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -1376,10 +1376,23 @@
           MP->unorderedDeleteIncomingBlock(BB);
           tryRemoveTrivialPhi(MP);
         }
-    // Drop all references of all accesses in BB
+
+    // Drop all references of all accesses in BB and remove those accesses from
+    // MemoryPhis outside of the dead blocks.
     if (MemorySSA::AccessList *Acc = MSSA->getWritableBlockAccesses(BB))
-      for (MemoryAccess &MA : *Acc)
+      for (MemoryAccess &MA : *Acc) {
         MA.dropAllReferences();
+
+        for (auto UI = MA.use_begin(), UE = MA.use_end(); UI != UE;) {
+          auto &U = *UI++;
+          auto *MemPhi = dyn_cast_or_null<MemoryPhi>(U.getUser());
+          if (!MemPhi || DeadBlocks.contains(MemPhi->getBlock()))
+            continue;
+          MemPhi->unorderedDeleteIncomingIf(
+              [&MA](MemoryAccess *Acc, BasicBlock *BB) { return &MA == Acc; });
+          tryRemoveTrivialPhi(MemPhi);
+        }
+      }
   }
 
   // Next, delete all memory accesses in each block


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87992.293035.patch
Type: text/x-patch
Size: 3184 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200920/347e103f/attachment.bin>