[PATCH] D48372: [MemorySSAUpdater] Remove deleted trivial Phis from active workset

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea created this revision.
labrinea added reviewers: llvm-commits, dberlin, efriedma, george.burgess.iv.
Herald added a subscriber: Prazek.

Bug fix for PR37808. The regression test is a reduced version of the original reproducer attached to the bug report. As stated in the report, the problem was that `InsertedPHIs` was keeping dangling pointers to deleted Memory-Phis. I've used AssertingVH to make sure we catch similar bugs earlier.


https://reviews.llvm.org/D48372

Files:
  include/llvm/Analysis/MemorySSAUpdater.h
  lib/Analysis/MemorySSAUpdater.cpp
  test/Transforms/GVNHoist/pr37808.ll


Index: test/Transforms/GVNHoist/pr37808.ll
===================================================================
--- /dev/null
+++ test/Transforms/GVNHoist/pr37808.ll
@@ -0,0 +1,40 @@
+; RUN: opt < %s -gvn-hoist -S | FileCheck %s
+
+define void @func() {
+; CHECK-LABEL: @func()
+; CHECK:       bb6:
+; CHECK:         store i64 0, i64* undef, align 8
+; CHECK:       bb7:
+; CHECK-NOT:     store i64 0, i64* undef, align 8
+; CHECK:       bb8:
+; CHECK-NOT:     store i64 0, i64* undef, align 8
+
+entry:
+  br label %bb1
+
+bb1:
+  br label %bb2
+
+bb2:
+  br label %bb3
+
+bb3:
+  br i1 undef, label %bb4, label %bb2
+
+bb4:
+  br i1 undef, label %bb5, label %bb3
+
+bb5:
+  br label %bb6
+
+bb6:
+  br i1 undef, label %bb7, label %bb8
+
+bb7:
+  store i64 0, i64* undef, align 8
+  unreachable
+
+bb8:
+  store i64 0, i64* undef, align 8
+  ret void
+}
Index: lib/Analysis/MemorySSAUpdater.cpp
===================================================================
--- lib/Analysis/MemorySSAUpdater.cpp
+++ lib/Analysis/MemorySSAUpdater.cpp
@@ -100,7 +100,7 @@
         unsigned i = 0;
         for (auto *Pred : predecessors(BB))
           Phi->addIncoming(PhiOps[i++], Pred);
-        InsertedPHIs.push_back(Phi);
+        InsertedPHIs.insert(Phi);
       }
       Result = Phi;
     }
@@ -205,6 +205,7 @@
   if (Same == nullptr)
     return MSSA->getLiveOnEntryDef();
   if (Phi) {
+    InsertedPHIs.remove(Phi);
     Phi->replaceAllUsesWith(Same);
     removeMemoryAccess(Phi);
   }
@@ -317,7 +318,7 @@
     MSSA->renamePass(MD->getBlock(), FirstDef, Visited);
     // We just inserted a phi into this block, so the incoming value will become
     // the phi anyway, so it does not matter what we pass.
-    for (auto *MP : InsertedPHIs)
+    for (auto &MP : InsertedPHIs)
       MSSA->renamePass(MP->getBlock(), nullptr, Visited);
   }
 }
Index: include/llvm/Analysis/MemorySSAUpdater.h
===================================================================
--- include/llvm/Analysis/MemorySSAUpdater.h
+++ include/llvm/Analysis/MemorySSAUpdater.h
@@ -32,6 +32,7 @@
 #ifndef LLVM_ANALYSIS_MEMORYSSAUPDATER_H
 #define LLVM_ANALYSIS_MEMORYSSAUPDATER_H
 
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -60,7 +61,7 @@
 class MemorySSAUpdater {
 private:
   MemorySSA *MSSA;
-  SmallVector<MemoryPhi *, 8> InsertedPHIs;
+  SmallSetVector<AssertingVH<MemoryPhi>, 8> InsertedPHIs;
   SmallPtrSet<BasicBlock *, 8> VisitedBlocks;
   SmallSet<AssertingVH<MemoryPhi>, 8> NonOptPhis;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48372.152090.patch
Type: text/x-patch
Size: 2566 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180620/a631946c/attachment.bin>