[MemorySSA] Make Phi cleanups consistent.

[asbirlea]

Summary

Make Phi cleanups consistent: remove self as a trivial Phi and
recurse to potentially remove other trivial phis.

[asbirlea]

Make this purely a cleanup patch.

[george.burgess.iv]

Assuming the test in D66495 (in part) covers this, LGTM with an optional nit.

Thanks!

lib/Analysis/MemorySSAUpdater.cpp

490

do we have many more calls like this laying around?

if so, would it be good to add a super simple overload that just takes a MPhi, asserts it's non-null, and expands to tryRemoveTrivialPhi(MPhi, MPhi->operands());?

[asbirlea]

Address comment.

lib/Analysis/MemorySSAUpdater.cpp

while (!Worklist.empty()) {
       }
     }
   }
 }
 
 void MemorySSAUpdater::removeEdge(BasicBlock *From, BasicBlock *To) {
   if (MemoryPhi *MPhi = MSSA->getMemoryAccess(To)) {
     MPhi->unorderedDeleteIncomingBlock(From);
-    if (MPhi->getNumIncomingValues() == 1)
-      removeMemoryAccess(MPhi);
+    auto OperRange = MPhi->operands();
+    tryRemoveTrivialPhi(MPhi, OperRange);
   }
 }
 
 void MemorySSAUpdater::removeDuplicatePhiEdgesBetween(const BasicBlock *From,
                                                       const BasicBlock *To) {
   if (MemoryPhi *MPhi = MSSA->getMemoryAccess(To)) {
     bool Found = false;
     MPhi->unorderedDeleteIncomingIf([&](const MemoryAccess *, BasicBlock *B) {
       if (From != B)
         return false;
       if (Found)
         return true;
       Found = true;
       return false;
     });
-    if (MPhi->getNumIncomingValues() == 1)
-      removeMemoryAccess(MPhi);
+    auto OperRange = MPhi->operands();
+    tryRemoveTrivialPhi(MPhi, OperRange);
   }
 }
 
 static MemoryAccess *getNewDefiningAccessForClone(MemoryAccess *MA,
                                                   const ValueToValueMapTy &VMap,
                                                   PhiToDefMap &MPhiMap,
                                                   bool CloneWasSimplified,
                                                   MemorySSA *MSSA) {
void MemorySSAUpdater::updatePhisWhenInsertingUniqueBackedgeBlock(
   MPhi->setIncomingValue(0, AccFromPreheader);
   MPhi->setIncomingBlock(0, Preheader);
   for (unsigned I = MPhi->getNumIncomingValues() - 1; I >= 1; --I)
     MPhi->unorderedDeleteIncoming(I);
   MPhi->addIncoming(NewMPhi, BEBlock);
 
   // If NewMPhi is a trivial phi, remove it. Its use in the header MPhi will be
   // replaced with the unique value.
-  if (HasUniqueIncomingValue)
-    removeMemoryAccess(NewMPhi);
+  auto OperRange = MPhi->operands();
+  tryRemoveTrivialPhi(MPhi, OperRange);
 }
 
 void MemorySSAUpdater::updateForClonedLoop(const LoopBlocksRPO &LoopBlocks,
                                            ArrayRef<BasicBlock *> ExitBlocks,
                                            const ValueToValueMapTy &VMap,
                                            bool IgnoreIncomingWithNoClones) {
   PhiToDefMap MPhiMap;
 
Phi->unorderedDeleteIncomingIf([&](MemoryAccess *MA, BasicBlock *B) {
         NewPhi->addIncoming(MA, B);
         if (!IdenticalEdgesWereMerged)
           PredsSet.erase(B);
         return true;
       }
       return false;
     });
     Phi->addIncoming(NewPhi, New);
-    if (onlySingleValue(NewPhi))
-      removeMemoryAccess(NewPhi);
+    auto OperRange = NewPhi->operands();
+    tryRemoveTrivialPhi(NewPhi, OperRange);
   }
 }
 
 void MemorySSAUpdater::removeMemoryAccess(MemoryAccess *MA, bool OptimizePhis) {
   assert(!MSSA->isLiveOnEntryDef(MA) &&
          "Trying to remove the live on entry def");
   // We can only delete phi nodes if they have no uses, or we can replace all
   // uses with a single definition.
void MemorySSAUpdater::removeBlocks(
   // First delete all uses of BB in MemoryPhis.
   for (BasicBlock *BB : DeadBlocks) {
     Instruction *TI = BB->getTerminator();
     assert(TI && "Basic block expected to have a terminator instruction");
     for (BasicBlock *Succ : successors(TI))
       if (!DeadBlocks.count(Succ))
         if (MemoryPhi *MP = MSSA->getMemoryAccess(Succ)) {
           MP->unorderedDeleteIncomingBlock(BB);
-          if (MP->getNumIncomingValues() == 1)
-            removeMemoryAccess(MP);
+          auto OperRange = MP->operands();
+          tryRemoveTrivialPhi(MP, OperRange);
         }
     // Drop all references of all accesses in BB
     if (MemorySSA::AccessList *Acc = MSSA->getWritableBlockAccesses(BB))
       for (MemoryAccess &MA : *Acc)
         MA.dropAllReferences();
   }
 
   // Next, delete all memory accesses in each block

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
+}