Push isDereferenceableAndAlignedPointer down into isSafeToLoadUnconditionally

[apilipenko]

Summary

Separated from D10920.

isSafeToLoadUnconditionally must take alignment into account. There are tests which check this behavior. That's why isDereferenceableAndAlignedPointer must be used instead of isDereferenceablePointer.

As a side effect optimizations which use isSafeToLoadUnconditionally will benefit from dereferenceable attribute. Corresponding test cases added.

[reames]

LGTM w/requested test.

test/Transforms/InstCombine/select.ll

1311

Please add a test case that shows a argument which is not sufficiently dereferenceable. AKA, a negative test.

lib/Analysis/Loads.cpp

bool llvm::isSafeToLoadUnconditionally(Value *V, unsigned Align,
                                        Instruction *ScanFrom) {
   const DataLayout &DL = ScanFrom->getModule()->getDataLayout();
 
   // Zero alignment means that the load has the ABI alignment for the target
   if (Align == 0)
     Align = DL.getABITypeAlignment(V->getType()->getPointerElementType());
   assert(isPowerOf2_32(Align));
 
+  if (isDereferenceableAndAlignedPointer(V, Align, DL))
+    return true;
+
   int64_t ByteOffset = 0;
   Value *Base = V;
   Base = GetPointerBaseWithConstantOffset(V, ByteOffset, DL);
 
   if (ByteOffset < 0) // out of bounds
     return false;
 
   Type *BaseType = nullptr;

lib/Transforms/Scalar/SROA.cpp

for (unsigned Idx = 0, Num = PN.getNumIncomingValues(); Idx != Num; ++Idx) {
     // If the predecessor has a single successor, then the edge isn't
     // critical.
     if (TI->getNumSuccessors() == 1)
       continue;
 
     // If this pointer is always safe to load, or if we can prove that there
     // is already a load in the block, then we can move the load to the pred
     // block.
-    if (isDereferenceablePointer(InVal, DL) ||
-        isSafeToLoadUnconditionally(InVal, MaxAlign, TI))
+    if (isSafeToLoadUnconditionally(InVal, MaxAlign, TI))
       continue;
 
     return false;
   }
 
   return true;
 }
 

 ///   %V = select i1 %cond, i32 %V1, i32 %V2
 ///
 /// We can do this to a select if its only uses are loads and if the operand
 /// to the select can be loaded unconditionally.
 static bool isSafeSelectToSpeculate(SelectInst &SI) {
   Value *TValue = SI.getTrueValue();
   Value *FValue = SI.getFalseValue();
   const DataLayout &DL = SI.getModule()->getDataLayout();
-  bool TDerefable = isDereferenceablePointer(TValue, DL);
-  bool FDerefable = isDereferenceablePointer(FValue, DL);
 
   for (User *U : SI.users()) {
     LoadInst *LI = dyn_cast<LoadInst>(U);
     if (!LI || !LI->isSimple())
       return false;
 
     // Both operands to the select need to be dereferencable, either
     // absolutely (e.g. allocas) or at this point because we can see other
     // accesses to it.
-    if (!TDerefable &&
-        !isSafeToLoadUnconditionally(TValue, LI->getAlignment(), LI))
+    if (!isSafeToLoadUnconditionally(TValue, LI->getAlignment(), LI))
       return false;
-    if (!FDerefable &&
-        !isSafeToLoadUnconditionally(FValue, LI->getAlignment(), LI))
+    if (!isSafeToLoadUnconditionally(FValue, LI->getAlignment(), LI))
       return false;
   }
 
   return true;
 }
 
 static void speculateSelectInstLoads(SelectInst &SI) {
   DEBUG(dbgs() << "    original: " << SI << "\n");

lib/Transforms/Scalar/ScalarReplAggregates.cpp

 ///   %V1 = load i32* %Alloca      -> will be mem2reg'd
 ///   %V2 = load i32* %Other
 ///   %V = select i1 %cond, i32 %V1, i32 %V2
 ///
 /// We can do this to a select if its only uses are loads and if the operand to
 /// the select can be loaded unconditionally.
 static bool isSafeSelectToSpeculate(SelectInst *SI) {
   const DataLayout &DL = SI->getModule()->getDataLayout();
-  bool TDerefable = isDereferenceablePointer(SI->getTrueValue(), DL);
-  bool FDerefable = isDereferenceablePointer(SI->getFalseValue(), DL);
 
   for (User *U : SI->users()) {
     LoadInst *LI = dyn_cast<LoadInst>(U);
     if (!LI || !LI->isSimple()) return false;
 
     // Both operands to the select need to be dereferencable, either absolutely
     // (e.g. allocas) or at this point because we can see other accesses to it.
-    if (!TDerefable &&
-        !isSafeToLoadUnconditionally(SI->getTrueValue(), LI->getAlignment(),
+    if (!isSafeToLoadUnconditionally(SI->getTrueValue(), LI->getAlignment(),
                                      LI))
       return false;
-    if (!FDerefable &&
-        !isSafeToLoadUnconditionally(SI->getFalseValue(), LI->getAlignment(),
+    if (!isSafeToLoadUnconditionally(SI->getFalseValue(), LI->getAlignment(),
                                      LI))
       return false;
   }
 
   return true;
 }
 
 /// isSafePHIToSpeculate - PHI instructions that use an alloca and are
if (Pred->getTerminator() == InVal)
       return false;
 
     // If the predecessor has a single successor, then the edge isn't critical.
     if (Pred->getTerminator()->getNumSuccessors() == 1)
       continue;
 
     // If this pointer is always safe to load, or if we can prove that there is
     // already a load in the block, then we can move the load to the pred block.
-    if (isDereferenceablePointer(InVal, DL) ||
-        isSafeToLoadUnconditionally(InVal, MaxAlign, Pred->getTerminator()))
+    if (isSafeToLoadUnconditionally(InVal, MaxAlign, Pred->getTerminator()))
       continue;
 
     return false;
   }
 
   return true;
 }
 

test/Transforms/InstCombine/select.ll

entry:
   store i32 0, i32* %y
   ; Block forwarding by storing to %z which could alias either %x or %y.
   store i32 42, i32* %z
   %p = select i1 %flag, i32* %x, i32* %y
   %v = load i32, i32* %p
   ret i32 %v
 }
 
+define i32 @test78_deref(i1 %flag, i32* dereferenceable(4) %x, i32* dereferenceable(4) %y, i32* %z) {
+; Test that we can speculate the loads around the select even when we can't
+; fold the load completely away.
+; CHECK-LABEL: @test78_deref(
+; CHECK:         %[[V1:.*]] = load i32, i32* %x
+; CHECK-NEXT:    %[[V2:.*]] = load i32, i32* %y
+; CHECK-NEXT:    %[[S:.*]] = select i1 %flag, i32 %[[V1]], i32 %[[V2]]
+; CHECK-NEXT:    ret i32 %[[S]]
+entry:
+  %p = select i1 %flag, i32* %x, i32* %y
+  %v = load i32, i32* %p
+  ret i32 %v
+}
+
 define i32 @test78_neg(i1 %flag, i32* %x, i32* %y, i32* %z) {
 ; The same as @test78 but we can't speculate the load because it can trap
 ; if under-aligned.
 ; CHECK-LABEL: @test78_neg(
 ; CHECK: %p = select i1 %flag, i32* %x, i32* %y
 ; CHECK-NEXT: %v = load i32, i32* %p, align 16
 ; CHECK-NEXT: ret i32 %v
 entry:

test/Transforms/TailCallElim/reorder_load.ll

 recurse:    ; preds = %else
   %tmp7 = add i32 %start_arg, 1   ; <i32> [#uses=1]
   %first = load i32, i32* %a_arg    ; <i32> [#uses=1]
   %tmp8 = call fastcc i32 @raise_load_4(i32* %a_arg, i32 %first, i32 %tmp7)    ; <i32> [#uses=1]
   %second = load i32, i32* %a_arg    ; <i32> [#uses=1]
   %tmp10 = add i32 %second, %tmp8   ; <i32> [#uses=1]
   ret i32 %tmp10
 }
+
+; This load can be moved above the call because the function won't write to it
+; and the a_arg is dereferenceable.
+define fastcc i32 @raise_load_5(i32* dereferenceable(4) %a_arg, i32 %a_len_arg, i32 %start_arg) readonly {
+; CHECK-LABEL: @raise_load_5(
+; CHECK-NOT: call
+; CHECK: load i32, i32*
+; CHECK-NOT: call
+; CHECK: }
+entry:
+  %tmp2 = icmp sge i32 %start_arg, %a_len_arg    ; <i1> [#uses=1]
+  br i1 %tmp2, label %if, label %else
+
+if:   ; preds = %entry
+  ret i32 0
+
+else:   ; preds = %entry
+  %tmp7 = add i32 %start_arg, 1   ; <i32> [#uses=1]
+  %tmp8 = call fastcc i32 @raise_load_5(i32* %a_arg, i32 %a_len_arg, i32 %tmp7)    ; <i32> [#uses=1]
+  %tmp9 = load i32, i32* %a_arg    ; <i32> [#uses=1]
+  %tmp10 = add i32 %tmp9, %tmp8   ; <i32> [#uses=1]
+  ret i32 %tmp10
+}