[PATCH] D63147: [MemorySSA] Add additional verification for phis.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea created this revision.
asbirlea added a reviewer: george.burgess.iv.
Herald added subscribers: Prazek, jlebar.
Herald added a project: LLVM.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D63147

Files:
  include/llvm/Analysis/MemorySSA.h
  lib/Analysis/MemorySSA.cpp
  lib/Analysis/MemorySSAUpdater.cpp


Index: lib/Analysis/MemorySSAUpdater.cpp
===================================================================
--- lib/Analysis/MemorySSAUpdater.cpp
+++ lib/Analysis/MemorySSAUpdater.cpp
@@ -1031,7 +1031,7 @@
 
   // 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));
 
Index: lib/Analysis/MemorySSA.cpp
===================================================================
--- lib/Analysis/MemorySSA.cpp
+++ lib/Analysis/MemorySSA.cpp
@@ -1850,6 +1850,7 @@
   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
@@ -1862,6 +1863,33 @@
   // example, see test4 added in D51960.
 }
 
+void MemorySSA::verifyPrevDefInPhis(Function &F) const {
+  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);
+        while (true) {
+          if (auto *DefList = getBlockDefs(Pred)) {
+            auto *LastAcc = &*(--DefList->end());
+            assert(LastAcc == IncAcc && "Incorrect incoming access into phi.");
+            (void)LastAcc;
+            break;
+          }
+          if (auto *DTNode = DT->getNode(Pred))
+            if (auto *IDom = DTNode->getIDom()) {
+              Pred = IDom->getBlock();
+              continue;
+            }
+          assert(IncAcc == getLiveOnEntryDef() && "Expected LoE inc");
+          (void)IncAcc;
+          break;
+        }
+      }
+    }
+  }
+}
+
 /// 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 {
Index: include/llvm/Analysis/MemorySSA.h
===================================================================
--- include/llvm/Analysis/MemorySSA.h
+++ include/llvm/Analysis/MemorySSA.h
@@ -793,6 +793,7 @@
   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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63147.204093.patch
Type: text/x-patch
Size: 2592 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190611/7850e489/attachment.bin>