[MemorySSA] Add additional verification for phis.

Summary:
Verify that the incoming defs into phis are the last defs from the
respective incoming blocks.
When moving blocks, insertDef must RenameUses. Adding this verification
makes GVNHoist tests fail that uncovered this issue.

Reviewers: george.burgess.iv

Subscribers: jlebar, Prazek, llvm-commits

Tags: #llvm

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

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

diff --git a/include/llvm/Analysis/MemorySSA.h b/include/llvm/Analysis/MemorySSA.h
index 282290b899badf2421f3a6cfb515ef87bea30554..e89bf26a7234b9f5f9d1b873364d1dea5062de92 100644 (file)
--- a/include/llvm/Analysis/MemorySSA.h
+++ b/include/llvm/Analysis/MemorySSA.h
@@ -793,6 +793,7 @@ protected:
   friend class MemorySSAPrinterLegacyPass;
   friend class MemorySSAUpdater;
 
+  void verifyPrevDefInPhis(Function &F) const;
   void verifyDefUses(Function &F) const;
   void verifyDomination(Function &F) const;
   void verifyOrdering(Function &F) const;

diff --git a/lib/Analysis/MemorySSA.cpp b/lib/Analysis/MemorySSA.cpp
index 5df56ddd8e3b1b749c2418182239a6a60a5bca6a..ed4b645623288bd0fb09af22a8a148b231073767 100644 (file)
--- a/lib/Analysis/MemorySSA.cpp
+++ b/lib/Analysis/MemorySSA.cpp
@@ -49,6 +49,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
+#include <cstdlib>
 #include <iterator>
 #include <memory>
 #include <utility>
@@ -1852,6 +1853,7 @@ void MemorySSA::verifyMemorySSA() const {
   verifyDomination(F);
   verifyOrdering(F);
   verifyDominationNumbers(F);
+  verifyPrevDefInPhis(F);
   // Previously, the verification used to also verify that the clobberingAccess
   // cached by MemorySSA is the same as the clobberingAccess found at a later
   // query to AA. This does not hold true in general due to the current fragility
@@ -1864,6 +1866,46 @@ void MemorySSA::verifyMemorySSA() const {
   // example, see test4 added in D51960.
 }
 
+void MemorySSA::verifyPrevDefInPhis(Function &F) const {
+#ifndef NDEBUG
+  for (const BasicBlock &BB : F) {
+    if (MemoryPhi *Phi = getMemoryAccess(&BB)) {
+      for (unsigned I = 0, E = Phi->getNumIncomingValues(); I != E; ++I) {
+        auto *Pred = Phi->getIncomingBlock(I);
+        auto *IncAcc = Phi->getIncomingValue(I);
+        // If Pred has no unreachable predecessors, get last def looking at
+        // IDoms. If, while walkings IDoms, any of these has an unreachable
+        // predecessor, then the expected incoming def is LoE.
+        if (auto *DTNode = DT->getNode(Pred)) {
+          while (DTNode) {
+            if (auto *DefList = getBlockDefs(DTNode->getBlock())) {
+              auto *LastAcc = &*(--DefList->end());
+              assert(LastAcc == IncAcc &&
+                     "Incorrect incoming access into phi.");
+              break;
+            }
+            DTNode = DTNode->getIDom();
+          }
+          assert((DTNode || IncAcc == getLiveOnEntryDef()) &&
+                 "Expected LoE inc");
+        } else if (auto *DefList = getBlockDefs(Pred)) {
+          // If Pred has unreachable predecessors, but has at least a Def, the
+          // incoming access can be the last Def in Pred, or it could have been
+          // optimized to LoE.
+          auto *LastAcc = &*(--DefList->end());
+          assert((LastAcc == IncAcc || IncAcc == getLiveOnEntryDef()) &&
+                 "Incorrect incoming access into phi.");
+        } else {
+          // If Pred has unreachable predecessors and no Defs, incoming access
+          // should be LoE.
+          assert(IncAcc == getLiveOnEntryDef() && "Expected LoE inc");
+        }
+      }
+    }
+  }
+#endif
+}
+
 /// Verify that all of the blocks we believe to have valid domination numbers
 /// actually have valid domination numbers.
 void MemorySSA::verifyDominationNumbers(const Function &F) const {

diff --git a/lib/Analysis/MemorySSAUpdater.cpp b/lib/Analysis/MemorySSAUpdater.cpp
index 42bbe7483b28dfb99a4dc34179de079c724a2262..7fea41a8a534f3537c9628d983abfd2516580ffc 100644 (file)
--- a/lib/Analysis/MemorySSAUpdater.cpp
+++ b/lib/Analysis/MemorySSAUpdater.cpp
@@ -1074,7 +1074,7 @@ void MemorySSAUpdater::moveTo(MemoryUseOrDef *What, BasicBlock *BB,
 
   // Now reinsert it into the IR and do whatever fixups needed.
   if (auto *MD = dyn_cast<MemoryDef>(What))
-    insertDef(MD);
+    insertDef(MD, true);
   else
     insertUse(cast<MemoryUse>(What));
 

diff --git a/test/Analysis/MemorySSA/unreachable.ll b/test/Analysis/MemorySSA/unreachable.ll
new file mode 100644 (file)
index 0000000..6a93643
--- /dev/null
+++ b/test/Analysis/MemorySSA/unreachable.ll
@@ -0,0 +1,31 @@
+; RUN: opt -licm -enable-mssa-loop-dependency -verify-memoryssa %s -S | FileCheck %s
+; REQUIRES: asserts
+; Ensure verification doesn't fail with unreachable blocks.
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
+declare dso_local void @f()
+
+; CHECK-LABEL: @foo
+define dso_local void @foo() {
+entry:
+  br i1 undef, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  br label %try.cont
+
+if.end:                                           ; preds = %entry
+; 1 = MemoryDef(liveOnEntry)
+  call void @f()
+  br label %try.cont
+
+catch:                                            ; No predecessors!
+; 2 = MemoryDef(liveOnEntry)
+  call void @f()
+  br label %try.cont
+
+try.cont:                                         ; preds = %if.end, %catch, %if.then
+; 3 = MemoryPhi({if.then,liveOnEntry},{if.end,1},{catch,liveOnEntry})
+  ret void
+}