[PATCH] D33044: [NewGVN] Fix verification of MemoryPhis in verifyMemoryCongruency()

[Davide Italiano via Phabricator via llvm-commits]

davide created this revision.
Herald added a subscriber: Prazek.

`verifyMemoryCongruency()` filters out trivially dead MemoryDef, as we find them immediately dead, before moving from TOP to a new congruence class.
If you consider the testcase attached, I think we may have the same problem for PHI, and I think we may want to skip MemoryPhis if all the operands are dead.

This is the MemorySSA dump

  0 = MemoryDef(liveOnEntry)
  1 = MemoryDef(liveOnEntry)
  2 = MemoryDef(1)
  3 = MemoryPhi({entry,1},{body,2})

Looking at the operands of 3, they're both uses of trivially dead defs.

  Skipping trivially dead instruction   call void @llvm.lifetime.start.p0i8(i64 4, i8* undef)
  Marking   call void @llvm.lifetime.start.p0i8(i64 4, i8* undef) for deletion
  Skipping trivially dead instruction   call void @llvm.lifetime.start.p0i8(i64 4, i8* undef)
  Marking   call void @llvm.lifetime.start.p0i8(i64 4, i8* undef) for deletion

so we never move them out of TOP, and the phi is dead as well. Otherwise, we think this is reachable, and we hit an assertion.


https://reviews.llvm.org/D33044

Files:
  lib/Transforms/Scalar/NewGVN.cpp
  test/Transforms/NewGVN/verify-memoryphi.ll


Index: test/Transforms/NewGVN/verify-memoryphi.ll
===================================================================
--- /dev/null
+++ test/Transforms/NewGVN/verify-memoryphi.ll
@@ -0,0 +1,29 @@
+; Skip dead MemoryPhis when performing memory congruency verification
+; in NewGVN.
+; RUN: opt -S -newgvn %s | FileCheck %s
+; REQUIRES: asserts
+
+; CHECK: define void @tinkywinky() {
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   br i1 false, label %body, label %end
+; CHECK:      body:
+; CHECK-NEXT:   store i8 undef, i8* null
+; CHECK-NEXT:   br label %end
+; CHECK:      end:
+; CHECK-NEXT:   ret void
+; CHECK-NEXT: }
+
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
+
+define void @tinkywinky() {
+entry:
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* undef)
+  br i1 false, label %body, label %end
+
+body:
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* undef)
+  br label %end
+
+end:
+  ret void
+}
Index: lib/Transforms/Scalar/NewGVN.cpp
===================================================================
--- lib/Transforms/Scalar/NewGVN.cpp
+++ lib/Transforms/Scalar/NewGVN.cpp
@@ -2548,6 +2548,19 @@
           return false;
         if (auto *MemDef = dyn_cast<MemoryDef>(Pair.first))
           return !isInstructionTriviallyDead(MemDef->getMemoryInst());
+
+        // We could have phi nodes which operands are all trivially dead,
+        // so we don't process them.
+        if (auto *MemPHI = dyn_cast<MemoryPhi>(Pair.first)) {
+          for (auto &U : MemPHI->incoming_values()) {
+            if (Instruction *I = dyn_cast<Instruction>(U.get())) {
+              if (!isInstructionTriviallyDead(I))
+                return true;
+            }
+          }
+          return false;
+        }
+
         return true;
       };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33044.98460.patch
Type: text/x-patch
Size: 1765 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170510/6456bc5c/attachment.bin>