[MemorySSA] Update Phi insertion.

Summary:
MemoryPhis may be needed following a Def insertion inthe IDF of all the
new accesses added (phis + potentially a def). Ensure this also  occurs when
only the new MemoryPhis are the defining accesses.

Note: The need for computing IDF here is because of new Phis added with
edges incoming from unreachable code, Phis that had previously been
simplified. The preferred solution is to not reintroduce such Phis.
This patch is the needed fix while working on the preferred solution.

Reviewers: george.burgess.iv

Subscribers: Prazek, sanjoy.google, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67927

llvm-svn: 372673

1 files changed, 39 insertions, 43 deletions

diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index 94417dc551a..bec811d17e2 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -313,10 +313,10 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) {
   // and that def is now our defining access.
   MD->setDefiningAccess(DefBefore);
 
-  // Remember the index where we may insert new phis below.
-  unsigned NewPhiIndex = InsertedPHIs.size();
-
   SmallVector<WeakVH, 8> FixupList(InsertedPHIs.begin(), InsertedPHIs.end());
+
+  SmallPtrSet<BasicBlock *, 2> DefiningBlocks;
+
   if (!DefBeforeSameBlock) {
     // If there was a local def before us, we must have the same effect it
     // did. Because every may-def is the same, any phis/etc we would create, it
@@ -335,53 +335,49 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) {
     auto Iter = MD->getDefsIterator();
     ++Iter;
     auto IterEnd = MSSA->getBlockDefs(MD->getBlock())->end();
-    if (Iter == IterEnd) {
-      ForwardIDFCalculator IDFs(*MSSA->DT);
-      SmallVector<BasicBlock *, 32> IDFBlocks;
-      SmallPtrSet<BasicBlock *, 2> DefiningBlocks;
-      for (const auto &VH : InsertedPHIs)
-        if (const auto *RealPHI = cast_or_null<MemoryPhi>(VH))
-          DefiningBlocks.insert(RealPHI->getBlock());
+    if (Iter == IterEnd)
       DefiningBlocks.insert(MD->getBlock());
-      IDFs.setDefiningBlocks(DefiningBlocks);
-      IDFs.calculate(IDFBlocks);
-      SmallVector<AssertingVH<MemoryPhi>, 4> NewInsertedPHIs;
-      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().
-        // Existing Phis in IDF may need fixing as well, and potentially be
-        // trivial before this insertion, hence add all IDF Phis. See PR43044.
-        NonOptPhis.insert(MPhi);
-      }
 
-      for (auto &MPhi : NewInsertedPHIs) {
-        auto *BBIDF = MPhi->getBlock();
-        for (auto *Pred : predecessors(BBIDF)) {
-          DenseMap<BasicBlock *, TrackingVH<MemoryAccess>> CachedPreviousDef;
-          MPhi->addIncoming(getPreviousDefFromEnd(Pred, CachedPreviousDef),
-                            Pred);
-        }
-      }
+    FixupList.push_back(MD);
+  }
 
-      // Re-take the index where we're adding the new phis, because the above
-      // call to getPreviousDefFromEnd, may have inserted into InsertedPHIs.
-      NewPhiIndex = InsertedPHIs.size();
-      for (auto &MPhi : NewInsertedPHIs) {
-        InsertedPHIs.push_back(&*MPhi);
-        FixupList.push_back(&*MPhi);
-      }
+  ForwardIDFCalculator IDFs(*MSSA->DT);
+  SmallVector<BasicBlock *, 32> IDFBlocks;
+  for (const auto &VH : InsertedPHIs)
+    if (const auto *RealPHI = cast_or_null<MemoryPhi>(VH))
+      DefiningBlocks.insert(RealPHI->getBlock());
+  IDFs.setDefiningBlocks(DefiningBlocks);
+  IDFs.calculate(IDFBlocks);
+  SmallVector<AssertingVH<MemoryPhi>, 4> NewInsertedPHIs;
+  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(). Existing Phis in IDF may need fixing
+    // as well, and potentially be trivial before this insertion, hence add all
+    // IDF Phis. See PR43044.
+    NonOptPhis.insert(MPhi);
+  }
 
-    FixupList.push_back(MD);
+  for (auto &MPhi : NewInsertedPHIs) {
+    auto *BBIDF = MPhi->getBlock();
+    for (auto *Pred : predecessors(BBIDF)) {
+      DenseMap<BasicBlock *, TrackingVH<MemoryAccess>> CachedPreviousDef;
+      MPhi->addIncoming(getPreviousDefFromEnd(Pred, CachedPreviousDef), Pred);
+    }
   }
 
+  // Remember the index where we may insert new phis.
+  unsigned NewPhiIndex = InsertedPHIs.size();
+  for (auto &MPhi : NewInsertedPHIs) {
+    InsertedPHIs.push_back(&*MPhi);
+    FixupList.push_back(&*MPhi);
+  }
   // Remember the index where we stopped inserting new phis above, since the
   // fixupDefs call in the loop below may insert more, that are already minimal.
   unsigned NewPhiIndexEnd = InsertedPHIs.size();