[MemorySSA] Fix existing phis when inserting defs.

[asbirlea]

Summary

When inserting a new Def, and inserting Phis in the IDF when needed,
also mark the already existing Phis in the IDF as non-optimized, since
these may need fixing as well.
In the test attached, there is a Phi in the IDF that happens to be
trivial, and is wrongfully removed by the call to getLastDef that
follows. This is a valid situation and the existing IDF Phis need to
marked as "may need fixing" as well.
Resolves PR43044.

[george.burgess.iv]

Thanks!

lib/Analysis/MemorySSAUpdater.cpp

355 ↗

(On Diff #216236)

this is somewhat surprising to me, so please add to this comment some kind of pointer to PR43044 and/or test/Analysis/MemorySSA/PR43044.ll. that way, understanding why this happens is a bit easier for future interested parties :)

llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp

if (!DefBeforeSameBlock) {
     if (Iter == IterEnd) {
       ForwardIDFCalculator IDFs(*MSSA->DT);
       SmallVector<BasicBlock *, 32> IDFBlocks;
       SmallPtrSet<BasicBlock *, 2> DefiningBlocks;
       DefiningBlocks.insert(MD->getBlock());
       IDFs.setDefiningBlocks(DefiningBlocks);
       IDFs.calculate(IDFBlocks);
       SmallVector<AssertingVH<MemoryPhi>, 4> NewInsertedPHIs;
-      for (auto *BBIDF : IDFBlocks)
-        if (!MSSA->getMemoryAccess(BBIDF)) {
-          auto *MPhi = MSSA->createMemoryPhi(BBIDF);
+      for (auto *BBIDF : IDFBlocks) {
+        auto *MPhi = MSSA->getMemoryAccess(BBIDF);
+        if (!MPhi) {
+          MPhi = MSSA->createMemoryPhi(BBIDF);
           NewInsertedPHIs.push_back(MPhi);
-          // Add the phis created into the IDF blocks to NonOptPhis, so they are
-          // not optimized out as trivial by the call to getPreviousDefFromEnd
-          // below. Once they are complete, all these Phis are added to the
-          // FixupList, and removed from NonOptPhis inside fixupDefs().
-          NonOptPhis.insert(MPhi);
-        }
+        }
+        // Existing Phis in IDF may need fixing as well, and potentially be
+        // trivial before this insertion, hence add all IDF Phis. See PR43044.
 
       for (auto &MPhi : NewInsertedPHIs) {
         auto *BBIDF = MPhi->getBlock();
         for (auto *Pred : predecessors(BBIDF)) {
           DenseMap<BasicBlock *, TrackingVH<MemoryAccess>> CachedPreviousDef;
           MPhi->addIncoming(getPreviousDefFromEnd(Pred, CachedPreviousDef),
                             Pred);
         }

llvm/trunk/test/Analysis/MemorySSA/PR43044.ll

+; RUN: opt -loop-rotate -licm -enable-mssa-loop-dependency -verify-memoryssa %s -S | FileCheck %s
+; REQUIRES: asserts
+
+target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
+target triple = "s390x-ibm-linux"
+
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+; CHECK-LABEL: @func_42()
+define void @func_42() {
+entry:
+  br label %for.cond1050
+
+for.cond1050.loopexit:                            ; preds = %for.cond1373
+  br label %for.cond1050
+
+for.cond1050:                                     ; preds = %for.cond1050.loopexit, %entry
+  %storemerge6 = phi i32 [ 2, %entry ], [ 0, %for.cond1050.loopexit ]
+  %cmp1051 = icmp sgt i32 %storemerge6, -1
+  br i1 %cmp1051, label %for.cond1055.preheader, label %cleanup1400.loopexit1
+
+for.cond1055.preheader:                           ; preds = %for.cond1050
+  store i64 0, i64* null, align 8
+  %0 = load i64, i64* null, align 8
+  %tobool1383 = icmp eq i64 %0, 0
+  br i1 %tobool1383, label %for.cond1055.preheader.cleanup1400.loopexit.split_crit_edge, label %for.cond1055.preheader.for.cond1055.preheader.split_crit_edge
+
+for.cond1055.preheader.for.cond1055.preheader.split_crit_edge: ; preds = %for.cond1055.preheader
+  br label %for.body1376
+
+for.cond1055.preheader.cleanup1400.loopexit.split_crit_edge: ; preds = %for.cond1055.preheader
+  br label %cleanup1400.loopexit.split
+
+for.cond1373:                                     ; preds = %for.body1376
+  br i1 true, label %for.body1376, label %for.cond1050.loopexit
+
+for.body1376:                                     ; preds = %for.cond1373, %for.cond1055.preheader.for.cond1055.preheader.split_crit_edge
+  br i1 false, label %cleanup1400.loopexit, label %for.cond1373
+
+cleanup1400.loopexit:                             ; preds = %for.body1376
+  br label %cleanup1400.loopexit.split
+
+cleanup1400.loopexit.split:                       ; preds = %cleanup1400.loopexit, %for.cond1055.preheader.cleanup1400.loopexit.split_crit_edge
+  br label %cleanup1400
+
+cleanup1400.loopexit1:                            ; preds = %for.cond1050
+  br label %cleanup1400
+
+cleanup1400:                                      ; preds = %cleanup1400.loopexit1, %cleanup1400.loopexit.split
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull undef)
+  unreachable
+}