[MemorySSAUpdater] Mark Phi users of a node being moved as non-optimize

Fix PR36484, as suggested:

<quote>
during moves, mark the direct users of the erased things that were phis as "not to be optimized"
<quote>

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@329621 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Analysis/MemorySSAUpdater.h b/include/llvm/Analysis/MemorySSAUpdater.h
index 3f4ef06..1fbffb2 100644 (file)
--- a/include/llvm/Analysis/MemorySSAUpdater.h
+++ b/include/llvm/Analysis/MemorySSAUpdater.h
@@ -33,6 +33,7 @@
 #define LLVM_ANALYSIS_MEMORYSSAUPDATER_H
 
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/MemorySSA.h"
 #include "llvm/IR/BasicBlock.h"
@@ -61,6 +62,7 @@ private:
   MemorySSA *MSSA;
   SmallVector<MemoryPhi *, 8> InsertedPHIs;
   SmallPtrSet<BasicBlock *, 8> VisitedBlocks;
+  SmallSet<AssertingVH<MemoryPhi>, 8> NonOptPhis;
 
 public:
   MemorySSAUpdater(MemorySSA *MSSA) : MSSA(MSSA) {}

diff --git a/lib/Analysis/MemorySSAUpdater.cpp b/lib/Analysis/MemorySSAUpdater.cpp
index 0d1e647..57b9cc9 100644 (file)
--- a/lib/Analysis/MemorySSAUpdater.cpp
+++ b/lib/Analysis/MemorySSAUpdater.cpp
@@ -180,6 +180,10 @@ MemoryAccess *MemorySSAUpdater::recursePhi(MemoryAccess *Phi) {
 template <class RangeType>
 MemoryAccess *MemorySSAUpdater::tryRemoveTrivialPhi(MemoryPhi *Phi,
                                                     RangeType &Operands) {
+  // Bail out on non-opt Phis.
+  if (NonOptPhis.count(Phi))
+    return Phi;
+
   // Detect equal or self arguments
   MemoryAccess *Same = nullptr;
   for (auto &Op : Operands) {
@@ -320,6 +324,10 @@ void MemorySSAUpdater::fixupDefs(const SmallVectorImpl<MemoryAccess *> &Vars) {
     auto *Defs = MSSA->getWritableBlockDefs(NewDef->getBlock());
     auto DefIter = NewDef->getDefsIterator();
 
+    // The temporary Phi is being fixed, unmark it for not to optimize.
+    if (MemoryPhi *Phi = dyn_cast_or_null<MemoryPhi>(NewDef))
+      NonOptPhis.erase(Phi);
+
     // If there is a local def after us, we only have to rename that.
     if (++DefIter != Defs->end()) {
       cast<MemoryDef>(DefIter)->setDefiningAccess(NewDef);
@@ -379,6 +387,11 @@ void MemorySSAUpdater::fixupDefs(const SmallVectorImpl<MemoryAccess *> &Vars) {
 template <class WhereType>
 void MemorySSAUpdater::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
                               WhereType Where) {
+  // Mark MemoryPhi users of What not to be optimized.
+  for (auto *U : What->users())
+    if (MemoryPhi *PhiUser = dyn_cast_or_null<MemoryPhi>(U))
+      NonOptPhis.insert(PhiUser);
+
   // Replace all our users with our defining access.
   What->replaceAllUsesWith(What->getDefiningAccess());
 
@@ -390,6 +403,10 @@ void MemorySSAUpdater::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
     insertDef(MD);
   else
     insertUse(cast<MemoryUse>(What));
+
+  // Clear dangling pointers. We added all MemoryPhi users, but not all
+  // of them are removed by fixupDefs().
+  NonOptPhis.clear();
 }
 
 // Move What before Where in the MemorySSA IR.

diff --git a/test/Transforms/GVNHoist/non-trivial-phi.ll b/test/Transforms/GVNHoist/non-trivial-phi.ll
new file mode 100644 (file)
index 0000000..f77ad1b
--- /dev/null
+++ b/test/Transforms/GVNHoist/non-trivial-phi.ll
@@ -0,0 +1,34 @@
+; RUN: opt -gvn-hoist %s -S -o - | FileCheck %s
+
+; CHECK: store
+; CHECK-NOT: store
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define void @f(i8* %p) {
+entry:
+  switch i4 undef, label %if.then30 [
+    i4 4, label %if.end
+    i4 0, label %if.end
+  ]
+
+if.end:
+  br label %if.end19
+
+if.end19:
+  br i1 undef, label %e, label %e.thread
+
+e.thread:
+  store i8 0, i8* %p, align 4
+  br label %if.then30
+
+if.then30:
+  call void @g()
+  unreachable
+
+e:
+  store i8 0, i8* %p, align 4
+  unreachable
+}
+
+declare void @g()