[PATCH] D43770: [NewGVN] Update phi-of-ops def block when updating existing ValuePHI.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: davide, dberlin.
Herald added a subscriber: Prazek.

In case we update a ValuePHI node created earlier, we could update it
based on a different OpPHI which could be in a different block. I think
we need to update the TempToBlock mapping reflecting the new block,
otherwise we would end up placing the new phi node in a wrong block.

This problem is exposed by the test case in
https://bugs.llvm.org/show_bug.cgi?id=36504.

This patch fixes a slightly simpler problem than in the bug report. In
the bug's re-producer, the additional problem is that we are re-using a
ValuePHI node with to few incoming values for the new OpPHI. If this
patch makes sense, I will follow it up with a patch that creates a new
PHI node if the existing PHI node has a different number of incoming
values.


https://reviews.llvm.org/D43770

Files:
  lib/Transforms/Scalar/NewGVN.cpp
  test/Transforms/NewGVN/phi-of-ops-move-block.ll


Index: test/Transforms/NewGVN/phi-of-ops-move-block.ll
===================================================================
--- /dev/null
+++ test/Transforms/NewGVN/phi-of-ops-move-block.ll
@@ -0,0 +1,37 @@
+; RUN: opt < %s -newgvn -S | FileCheck %s
+
+ at g_20 = external global i32, align 4
+
+define void @test() {
+entry:
+  br label %bb1
+
+bb1:                                      ; preds = %critedge, %entry
+  %storemerge = phi i32 [ 0, %entry ], [ %add1, %critedge ]
+  store i32 %storemerge, i32* @g_20, align 4
+  %cmp0 = icmp eq i32 %storemerge, 0
+  br i1 %cmp0, label %lr.ph, label %critedge
+
+lr.ph:                                           ; preds = %bb1
+  %lv = load i64, i64* inttoptr (i64 16 to i64*), align 16
+  %cmp1 = icmp eq i64 %lv, 0
+  br i1 %cmp1, label %preheader.split, label %critedge
+
+preheader.split:                                 ; preds = %lr.ph, %preheader.split
+  br label %preheader.split
+
+critedge:                                        ; preds = %lr.ph, %bb1
+  %.05.lcssa = phi i32 [ 0, %bb1 ], [ -1, %lr.ph ]
+  %cmp2 = icmp ne i32 %.05.lcssa, 0
+  %brmerge = or i1 %cmp0, %cmp2
+  %add1 = add nsw i32 %storemerge, -1
+  br i1 %brmerge, label %bb1, label %end
+
+end:
+  ret void
+}
+
+; CHECK-LABEL: critedge:
+; CHECK-NEXT: %phiofops1 = phi i1 [ false, %bb1 ], [ true, %lr.ph ]
+; CHECK-NEXT: %phiofops = phi i1 [ %cmp0, %bb1 ], [ true, %lr.ph ]
+; CHECK: br i1 %phiofops, label %bb1, label %end
Index: lib/Transforms/Scalar/NewGVN.cpp
===================================================================
--- lib/Transforms/Scalar/NewGVN.cpp
+++ lib/Transforms/Scalar/NewGVN.cpp
@@ -2819,6 +2819,7 @@
       for (auto PHIOp : Ops)
         ValuePHI->addIncoming(PHIOp.first, PHIOp.second);
     } else {
+      TempToBlock[ValuePHI] = PHIBlock;
       unsigned int i = 0;
       for (auto PHIOp : Ops) {
         ValuePHI->setIncomingValue(i, PHIOp.first);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43770.135927.patch
Type: text/x-patch
Size: 1908 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180226/6ebb5003/attachment.bin>