[llvm] r372673 - [MemorySSA] Update Phi insertion.

Alina Sbirlea via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 23 16:50:16 PDT 2019


Author: asbirlea
Date: Mon Sep 23 16:50:16 2019
New Revision: 372673

URL: http://llvm.org/viewvc/llvm-project?rev=372673&view=rev
Log:
[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

Added:
    llvm/trunk/test/Analysis/MemorySSA/pr43317.ll
Modified:
    llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp

Modified: llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp?rev=372673&r1=372672&r2=372673&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp (original)
+++ llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp Mon Sep 23 16:50:16 2019
@@ -313,10 +313,10 @@ void MemorySSAUpdater::insertDef(MemoryD
   // 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(MemoryD
     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);
-        }
-      }
-
-      // 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);
-      }
-    }
 
     FixupList.push_back(MD);
   }
 
+  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);
+  }
+
+  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();

Added: llvm/trunk/test/Analysis/MemorySSA/pr43317.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr43317.ll?rev=372673&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/MemorySSA/pr43317.ll (added)
+++ llvm/trunk/test/Analysis/MemorySSA/pr43317.ll Mon Sep 23 16:50:16 2019
@@ -0,0 +1,32 @@
+; RUN: opt -S -licm -enable-mssa-loop-dependency=true < %s | FileCheck %s
+; REQUIRES: asserts
+ at v_274 = external dso_local global i64, align 1
+ at v_295 = external dso_local global i16, align 1
+ at v_335 = external dso_local global i32, align 1
+
+; CHECK-LABEL: @main()
+define dso_local void @main() {
+entry:
+  store i32 undef, i32* @v_335, align 1
+  br i1 undef, label %gate, label %exit
+
+nopredentry1:                                     ; No predecessors!
+  br label %preinfiniteloop
+
+nopredentry2:                                     ; No predecessors!
+  br label %gate
+
+gate:                                             ; preds = %nopredentry2, %entry
+  br i1 undef, label %preinfiniteloop, label %exit
+
+preinfiniteloop:                                  ; preds = %gate, %nopredentry1
+  br label %infiniteloop
+
+infiniteloop:                                     ; preds = %infiniteloop, %preinfiniteloop
+  store i16 undef, i16* @v_295, align 1
+  br label %infiniteloop
+
+exit:                                             ; preds = %gate, %entry
+  store i64 undef, i64* @v_274, align 1
+  ret void
+}




More information about the llvm-commits mailing list