From 722339e40562003b4d32693fa568e98ab38da831 Mon Sep 17 00:00:00 2001
From: Serguei Katkov <serguei.katkov@azul.com>
Date: Thu, 9 Nov 2017 06:02:18 +0000
Subject: [PATCH] [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

llvm-svn: 317768
---
 llvm/lib/Transforms/Scalar/GVN.cpp             |  7 +++-
 llvm/test/Transforms/GVN/PRE/pre-jt-add.ll     | 36 ++++++++++++++++++
 llvm/test/Transforms/GVN/PRE/pre-poison-add.ll | 52 ++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100644 llvm/test/Transforms/GVN/PRE/pre-jt-add.ll
 create mode 100644 llvm/test/Transforms/GVN/PRE/pre-poison-add.ll

diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 6dcde95..b031f90 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -2288,9 +2288,12 @@ bool GVN::performScalarPRE(Instruction *CurInst) {
       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);
   }
 
diff --git a/llvm/test/Transforms/GVN/PRE/pre-jt-add.ll b/llvm/test/Transforms/GVN/PRE/pre-jt-add.ll
new file mode 100644
index 0000000..469b78c
--- /dev/null
+++ b/llvm/test/Transforms/GVN/PRE/pre-jt-add.ll
@@ -0,0 +1,36 @@
+; RUN: opt < %s -gvn -enable-pre -jump-threading -S | FileCheck %s
+
+@H = common global i32 0
+@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
+}
+
diff --git a/llvm/test/Transforms/GVN/PRE/pre-poison-add.ll b/llvm/test/Transforms/GVN/PRE/pre-poison-add.ll
new file mode 100644
index 0000000..61dfda5
--- /dev/null
+++ b/llvm/test/Transforms/GVN/PRE/pre-poison-add.ll
@@ -0,0 +1,52 @@
+; RUN: opt < %s -gvn -enable-pre -S | FileCheck %s
+
+@H = common global i32 0
+@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
+}
-- 
2.7.4