[llvm] r374471 - [MemorySSA] Update Phi simplification.

[Alina Sbirlea via llvm-commits]

Author: asbirlea
Date: Thu Oct 10 16:27:21 2019
New Revision: 374471

URL: http://llvm.org/viewvc/llvm-project?rev=374471&view=rev
Log:
[MemorySSA] Update Phi simplification.

When simplifying a Phi to the unique value found incoming, check that
there wasn't a Phi already created to break a cycle. If so, remove it.
Resolves PR43541.

Some additional nits included.

Added:
    llvm/trunk/test/Analysis/MemorySSA/pr43541.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=374471&r1=374470&r2=374471&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp (original)
+++ llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp Thu Oct 10 16:27:21 2019
@@ -44,15 +44,15 @@ MemoryAccess *MemorySSAUpdater::getPrevi
   // First, do a cache lookup. Without this cache, certain CFG structures
   // (like a series of if statements) take exponential time to visit.
   auto Cached = CachedPreviousDef.find(BB);
-  if (Cached != CachedPreviousDef.end()) {
+  if (Cached != CachedPreviousDef.end())
     return Cached->second;
-  }
 
   // If this method is called from an unreachable block, return LoE.
   if (!MSSA->DT->isReachableFromEntry(BB))
     return MSSA->getLiveOnEntryDef();
 
-  if (BasicBlock *Pred = BB->getSinglePredecessor()) {
+  if (BasicBlock *Pred = BB->getUniquePredecessor()) {
+    VisitedBlocks.insert(BB);
     // Single predecessor case, just recurse, we can only have one definition.
     MemoryAccess *Result = getPreviousDefFromEnd(Pred, CachedPreviousDef);
     CachedPreviousDef.insert({BB, Result});
@@ -96,9 +96,15 @@ MemoryAccess *MemorySSAUpdater::getPrevi
     // See if we can avoid the phi by simplifying it.
     auto *Result = tryRemoveTrivialPhi(Phi, PhiOps);
     // If we couldn't simplify, we may have to create a phi
-    if (Result == Phi && UniqueIncomingAccess && SingleAccess)
+    if (Result == Phi && UniqueIncomingAccess && SingleAccess) {
+      // A concrete Phi only exists if we created an empty one to break a cycle.
+      if (Phi) {
+        assert(Phi->operands().empty() && "Expected empty Phi");
+        Phi->replaceAllUsesWith(SingleAccess);
+        removeMemoryAccess(Phi);
+      }
       Result = SingleAccess;
-    else if (Result == Phi && !(UniqueIncomingAccess && SingleAccess)) {
+    } else if (Result == Phi && !(UniqueIncomingAccess && SingleAccess)) {
       if (!Phi)
         Phi = MSSA->createMemoryPhi(BB);
 
@@ -237,6 +243,7 @@ MemoryAccess *MemorySSAUpdater::tryRemov
 void MemorySSAUpdater::insertUse(MemoryUse *MU, bool RenameUses) {
   InsertedPHIs.clear();
   MU->setDefiningAccess(getPreviousDef(MU));
+
   // In cases without unreachable blocks, because uses do not create new
   // may-defs, there are only two cases:
   // 1. There was a def already below us, and therefore, we should not have

Added: llvm/trunk/test/Analysis/MemorySSA/pr43541.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr43541.ll?rev=374471&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/MemorySSA/pr43541.ll (added)
+++ llvm/trunk/test/Analysis/MemorySSA/pr43541.ll Thu Oct 10 16:27:21 2019
@@ -0,0 +1,50 @@
+; RUN: opt -gvn-hoist -enable-mssa-loop-dependency -S < %s | FileCheck %s
+; REQUIRES: asserts
+%struct.job_pool.6.7 = type { i32 }
+
+; CHECK-LABEL: @f()
+define dso_local void @f() {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.end, %entry
+  br label %for.body
+
+for.body:                                         ; preds = %for.cond
+  br label %if.end
+
+if.then:                                          ; No predecessors!
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %for.body
+  br i1 false, label %for.body12.lr.ph, label %for.end
+
+for.body12.lr.ph:                                 ; preds = %if.end
+  br label %for.body12
+
+for.body12:                                       ; preds = %if.end40, %for.body12.lr.ph
+  br label %if.then23
+
+if.then23:                                        ; preds = %for.body12
+  br i1 undef, label %if.then24, label %if.else
+
+if.then24:                                        ; preds = %if.then23
+  %0 = load %struct.job_pool.6.7*, %struct.job_pool.6.7** undef, align 8
+  br label %if.end40
+
+if.else:                                          ; preds = %if.then23
+  %1 = load %struct.job_pool.6.7*, %struct.job_pool.6.7** undef, align 8
+  br label %if.end40
+
+if.end40:                                         ; preds = %if.else, %if.then24
+  br i1 false, label %for.body12, label %for.cond9.for.end_crit_edge
+
+for.cond9.for.end_crit_edge:                      ; preds = %if.end40
+  br label %for.end
+
+for.end:                                          ; preds = %for.cond9.for.end_crit_edge, %if.end
+  br i1 true, label %if.then45, label %for.cond
+
+if.then45:                                        ; preds = %for.end
+  ret void
+}