[llvm] r317768 - [GVN PRE] Patch the source for Phi node in PRE

[Serguei Katkov via llvm-commits]

Author: skatkov
Date: Wed Nov  8 22:02:18 2017
New Revision: 317768

URL: http://llvm.org/viewvc/llvm-project?rev=317768&view=rev
Log:
[GVN PRE] Patch the source for Phi node in PRE

We must patch all existing incoming values of Phi node,
otherwise it is possible that we can see poison
where program does not expect to see it.

This is the similar what GVN does.

The added test test/Transforms/GVN/PRE/pre-jt-add.ll shows an
example of wrong optimization done by jump threading due to
GVN PRE did not patch existing incoming value.

Reviewers: mkazantsev, wmi, dberlin, davide
Reviewed By: dberlin
Subscribers: efriedma, llvm-commits
Differential Revision: https://reviews.llvm.org/D39637

Added:
    llvm/trunk/test/Transforms/GVN/PRE/pre-jt-add.ll
    llvm/trunk/test/Transforms/GVN/PRE/pre-poison-add.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=317768&r1=317767&r2=317768&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Wed Nov  8 22:02:18 2017
@@ -2288,9 +2288,12 @@ bool GVN::performScalarPRE(Instruction *
       PHINode::Create(CurInst->getType(), predMap.size(),
                       CurInst->getName() + ".pre-phi", &CurrentBlock->front());
   for (unsigned i = 0, e = predMap.size(); i != e; ++i) {
-    if (Value *V = predMap[i].first)
+    if (Value *V = predMap[i].first) {
+      // If we use an existing value in this phi, we have to patch the original
+      // value because the phi will be used to replace a later value.
+      patchReplacementInstruction(CurInst, V);
       Phi->addIncoming(V, predMap[i].second);
-    else
+    } else
       Phi->addIncoming(PREInstr, PREPred);
   }
 

Added: llvm/trunk/test/Transforms/GVN/PRE/pre-jt-add.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/pre-jt-add.ll?rev=317768&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/PRE/pre-jt-add.ll (added)
+++ llvm/trunk/test/Transforms/GVN/PRE/pre-jt-add.ll Wed Nov  8 22:02:18 2017
@@ -0,0 +1,36 @@
+; RUN: opt < %s -gvn -enable-pre -jump-threading -S | FileCheck %s
+
+ at H = common global i32 0
+ at G = common global i32 0
+
+define i32 @test(i1 %cond, i32 %v) nounwind {
+; CHECK-LABEL: @test
+entry:
+  br i1 %cond, label %bb, label %bb1
+
+bb:
+; CHECK: store
+; CHECK-NOT: br label %return
+  %add.1 = add nuw nsw i32 %v, -1
+  store i32 %add.1, i32* @G, align 4
+  br label %merge
+
+bb1:
+  br label %merge
+
+merge:
+  %add.2 = add i32 %v, -1
+  %cmp = icmp sgt i32 %add.2, 0
+  br i1 %cmp, label %action, label %return
+
+action:
+; CHECK: store
+; CHECK-NEXT: br label %return
+  store i32 %add.2, i32* @H, align 4
+  br label %return
+
+return:
+  %p = phi i32 [0, %merge], [1, %action]
+  ret i32 %p
+}
+

Added: llvm/trunk/test/Transforms/GVN/PRE/pre-poison-add.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/pre-poison-add.ll?rev=317768&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/PRE/pre-poison-add.ll (added)
+++ llvm/trunk/test/Transforms/GVN/PRE/pre-poison-add.ll Wed Nov  8 22:02:18 2017
@@ -0,0 +1,52 @@
+; RUN: opt < %s -gvn -enable-pre -S | FileCheck %s
+
+ at H = common global i32 0
+ at G = common global i32 0
+
+define i32 @test1(i1 %cond, i32 %v) nounwind {
+; CHECK-LABEL: @test1
+entry:
+    br i1 %cond, label %bb, label %bb1
+
+bb:
+    %add.1 = add nuw nsw i32 %v, 42
+; CHECK: %add.1 = add i32 %v, 42
+    store i32 %add.1, i32* @G, align 4
+    br label %return
+
+bb1:
+; CHECK: %.pre = add i32 %v, 42
+    br label %return
+
+return:
+; CHECK: %add.2.pre-phi = phi i32 [ %.pre, %bb1 ], [ %add.1, %bb ]
+; CHECK-NEXT: store i32 %add.2.pre-phi, i32* @H, align 4
+; CHECK-NEXT: ret i32 0
+    %add.2 = add i32 %v, 42
+    store i32 %add.2, i32* @H, align 4
+    ret i32 0
+}
+
+define i32 @test2(i1 %cond, i32 %v) nounwind {
+; CHECK-LABEL: @test2
+entry:
+    br i1 %cond, label %bb, label %bb1
+
+bb:
+    %add.1 = add i32 %v, 42
+; CHECK: %add.1 = add i32 %v, 42
+    store i32 %add.1, i32* @G, align 4
+    br label %return
+
+bb1:
+; CHECK: %.pre = add nuw nsw i32 %v, 42
+    br label %return
+
+return:
+; CHECK: %add.2.pre-phi = phi i32 [ %.pre, %bb1 ], [ %add.1, %bb ]
+; CHECK-NEXT: store i32 %add.2.pre-phi, i32* @H, align 4
+; CHECK-NEXT: ret i32 0
+    %add.2 = add nuw nsw i32 %v, 42
+    store i32 %add.2, i32* @H, align 4
+    ret i32 0
+}