[Sink] Really really fix predicate in legality check

[escha]

Summary

LoadInst isn't enough; we need to include intrinsics that perform loads too.

All side-effecting intrinsics and such are already covered by the isSafe check, so we just need to care about things that read from memory.

[loladiro]

Looks good to me with the minor tweak to the test. To keep the breadcrumb for future reference, I had changed this in D33179.

test/Transforms/Sink/badloadsink.ll

21

i32 %off or just get rid of the arguments entirely

[loladiro]

This was merged in rL322311, closing to get it off my dashboard. In the future, ideally add

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

with that syntax to the commit message, so Phabricator can close the revision automatically.

lib/Transforms/Scalar/Sink.cpp

static bool IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo,
 
   // If the block has multiple predecessors, this would introduce computation
   // on different code paths.  We could split the critical edge, but for now we
   // just punt.
   // FIXME: Split critical edges if not backedges.
   if (SuccToSinkTo->getUniquePredecessor() != Inst->getParent()) {
     // We cannot sink a load across a critical edge - there may be stores in
     // other code paths.
-    if (isa<LoadInst>(Inst))
+    if (Inst->mayReadFromMemory())
       return false;
 
     // We don't want to sink across a critical edge if we don't dominate the
     // successor. We could be introducing calculations to new code paths.
     if (!DT.dominates(Inst->getParent(), SuccToSinkTo))
       return false;
 
     // Don't sink instructions into a loop.

test/Transforms/Sink/badloadsink.ll

 ; RUN: opt < %s -basicaa -sink -S | FileCheck %s
 declare void @foo(i64 *)
-define i64 @sinkload(i1 %cmp) {
+declare i8* @llvm.load.relative.i32(i8* %ptr, i32 %offset) argmemonly nounwind readonly
+define i64 @sinkload(i1 %cmp, i8* %ptr, i32 %off) {
 ; CHECK-LABEL: @sinkload
 top:
     %a = alloca i64
 ; CHECK: call void @foo(i64* %a)
 ; CHECK-NEXT: %x = load i64, i64* %a
+; CHECK-NEXT: %y = call i8* @llvm.load.relative.i32(i8* %ptr, i32 %off)
     call void @foo(i64* %a)
     %x = load i64, i64* %a
+    %y = call i8* @llvm.load.relative.i32(i8* %ptr, i32 %off)
     br i1 %cmp, label %A, label %B
 A:
     store i64 0, i64 *%a
+    store i8 0, i8 *%ptr
     br label %B
 B:
 ; CHECK-NOT: load i64, i64 *%a
-    ret i64 %x
+; CHECK-NOT: call i8* @llvm.load.relative(i8* %ptr, i32 0)
+    %y2 = ptrtoint i8* %y to i64
+    %retval = add i64 %y2, %x
+    ret i64 %retval
 }
+