From: Max Kazantsev <max.kazantsev@azul.com>
Date: Tue, 28 Nov 2017 07:07:55 +0000 (+0000)
Subject: [GVN] Prevent ScalarPRE from hoisting across instructions that don't pass control... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f7019c8d319fc3995c2f28251190f27fd63ec333;p=llvm

[GVN] Prevent ScalarPRE from hoisting across instructions that don't pass control flow to successors

This is to address a problem similar to those in D37460 for Scalar PRE. We should not
PRE across an instruction that may not pass execution to its successor unless it is safe
to speculatively execute it.

Differential Revision: https://reviews.llvm.org/D38619


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@319147 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
index b031f905eba..76e295c1ad2 100644
--- a/lib/Transforms/Scalar/GVN.cpp
+++ b/lib/Transforms/Scalar/GVN.cpp
@@ -2255,6 +2255,20 @@ bool GVN::performScalarPRE(Instruction *CurInst) {
   Instruction *PREInstr = nullptr;
 
   if (NumWithout != 0) {
+    if (!isSafeToSpeculativelyExecute(CurInst)) {
+      // It is only valid to insert a new instruction if the current instruction
+      // is always executed. An instruction with implicit control flow could
+      // prevent us from doing it. If we cannot speculate the execution, then
+      // PRE should be prohibited.
+      auto It = FirstImplicitControlFlowInsts.find(CurrentBlock);
+      if (It != FirstImplicitControlFlowInsts.end()) {
+        assert(It->second->getParent() == CurrentBlock &&
+               "Implicit control flow map broken?");
+        if (OI->dominates(It->second, CurInst))
+          return false;
+      }
+    }
+
     // Don't do PRE across indirect branch.
     if (isa<IndirectBrInst>(PREPred->getTerminator()))
       return false;
diff --git a/test/Transforms/GVN/PRE/local-pre.ll b/test/Transforms/GVN/PRE/local-pre.ll
index 943f351f17a..22d9b506644 100644
--- a/test/Transforms/GVN/PRE/local-pre.ll
+++ b/test/Transforms/GVN/PRE/local-pre.ll
@@ -1,6 +1,13 @@
 ; RUN: opt < %s -gvn -enable-pre -S | FileCheck %s
 
+declare void @may_exit() nounwind
+
+declare void @may_exit_1(i32) nounwind
+
 define i32 @main(i32 %p, i32 %q) {
+
+; CHECK-LABEL: @main(
+
 block1:
     %cmp = icmp eq i32 %p, %q 
 	br i1 %cmp, label %block2, label %block3
@@ -20,3 +27,121 @@ block4:
 ; CHECK: %b.pre-phi = phi i32 [ %.pre, %block3 ], [ %a, %block2 ]
 ; CHECK-NEXT: ret i32 %b.pre-phi
 }
+
+; Don't PRE across implicit control flow.
+define i32 @test2(i32 %p, i32 %q) {
+
+; CHECK-LABEL: @test2
+; CHECK: block1:
+
+block1:
+  %cmp = icmp eq i32 %p, %q
+  br i1 %cmp, label %block2, label %block3
+
+block2:
+ %a = sdiv i32 %p, %q
+ br label %block4
+
+block3:
+  br label %block4
+
+; CHECK: block4:
+; CHECK-NEXT: call void @may_exit(
+; CHECK-NEXT: %b = sdiv
+; CHECK-NEXT: ret i32 %b
+
+block4:
+  call void @may_exit() nounwind
+  %b = sdiv i32 %p, %q
+  ret i32 %b
+}
+
+; Don't PRE across implicit control flow.
+define i32 @test3(i32 %p, i32 %q, i1 %r) {
+
+; CHECK-LABEL: @test3
+; CHECK: block1:
+
+block1:
+  br i1 %r, label %block2, label %block3
+
+block2:
+ %a = sdiv i32 %p, %q
+ br label %block4
+
+block3:
+  br label %block4
+
+block4:
+
+; CHECK: block4:
+; CHECK-NEXT: phi i32
+; CHECK-NEXT: call void @may_exit_1(
+; CHECK-NEXT: %b = sdiv
+; CHECK-NEXT: ret i32 %b
+
+  %phi = phi i32 [ 0, %block3 ], [ %a, %block2 ]
+  call void @may_exit_1(i32 %phi) nounwind
+  %b = sdiv i32 %p, %q
+  ret i32 %b
+
+}
+
+; It's OK to PRE an instruction that is guaranteed to be safe to execute
+; speculatively.
+; TODO: Does it make any sense in this case?
+define i32 @test4(i32 %p, i32 %q) {
+
+; CHECK-LABEL: @test4
+; CHECK: block1:
+
+block1:
+  %cmp = icmp eq i32 %p, %q
+  br i1 %cmp, label %block2, label %block3
+
+block2:
+ %a = sdiv i32 %p, 6
+ br label %block4
+
+block3:
+  br label %block4
+
+; CHECK: block4:
+; CHECK-NEXT: %b.pre-phi = phi i32
+; CHECK-NEXT: call void @may_exit(
+; CHECK-NEXT: ret i32 %b
+
+block4:
+  call void @may_exit() nounwind
+  %b = sdiv i32 %p, 6
+  ret i32 %b
+}
+
+; It is OK to PRE across implicit control flow if we don't insert new
+; instructions.
+define i32 @test5(i1 %cond, i32 %p, i32 %q) {
+
+; CHECK-LABEL: @test5
+; CHECK: block1:
+
+block1:
+  br i1 %cond, label %block2, label %block3
+
+block2:
+ %a = sdiv i32 %p, %q
+ br label %block4
+
+block3:
+  %b = sdiv i32 %p, %q
+  br label %block4
+
+; CHECK: block4:
+; CHECK-NEXT: %c.pre-phi = phi i32 [ %b, %block3 ], [ %a, %block2 ]
+; CHECK-NEXT: call void @may_exit()
+; CHECK-NEXT: ret i32 %c.pre-phi
+
+block4:
+  call void @may_exit() nounwind
+  %c = sdiv i32 %p, %q
+  ret i32 %c
+}