[llvm] r305102 - [Sink] Fix predicate in legality check

[Keno Fischer via llvm-commits]

Author: kfischer
Date: Fri Jun  9 14:31:10 2017
New Revision: 305102

URL: http://llvm.org/viewvc/llvm-project?rev=305102&view=rev
Log:
[Sink] Fix predicate in legality check

Summary:
isSafeToSpeculativelyExecute is the wrong predicate to use here.
All that checks for is whether it is safe to hoist a value due to
unaligned/un-dereferencable accesses. However, not only are we doing
sinking rather than hoisting, our concern is that the location
we're loading from may have been modified. Instead forbid sinking
any load across a critical edge.

Reviewers: majnemer

Subscribers: davide, llvm-commits

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

Added:
    llvm/trunk/test/Transforms/Sink/badloadsink.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/Sink.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/Sink.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Sink.cpp?rev=305102&r1=305101&r2=305102&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/Sink.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/Sink.cpp Fri Jun  9 14:31:10 2017
@@ -114,7 +114,7 @@ static bool IsAcceptableTarget(Instructi
   if (SuccToSinkTo->getUniquePredecessor() != Inst->getParent()) {
     // We cannot sink a load across a critical edge - there may be stores in
     // other code paths.
-    if (!isSafeToSpeculativelyExecute(Inst))
+    if (isa<LoadInst>(Inst))
       return false;
 
     // We don't want to sink across a critical edge if we don't dominate the

Added: llvm/trunk/test/Transforms/Sink/badloadsink.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Sink/badloadsink.ll?rev=305102&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Sink/badloadsink.ll (added)
+++ llvm/trunk/test/Transforms/Sink/badloadsink.ll Fri Jun  9 14:31:10 2017
@@ -0,0 +1,18 @@
+; RUN: opt < %s -basicaa -sink -S | FileCheck %s
+declare void @foo(i64 *)
+define i64 @sinkload(i1 %cmp) {
+; CHECK-LABEL: @sinkload
+top:
+    %a = alloca i64
+; CHECK: call void @foo(i64* %a)
+; CHECK-NEXT: %x = load i64, i64* %a
+    call void @foo(i64* %a)
+    %x = load i64, i64* %a
+    br i1 %cmp, label %A, label %B
+A:
+    store i64 0, i64 *%a
+    br label %B
+B:
+; CHECK-NOT: load i64, i64 *%a
+    ret i64 %x
+}