[llvm] r239849 - Reapply 239795 - [InstCombine] Propagate non-null facts to call parameters
Pete Cooper via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 25 11:09:56 PDT 2015
Hey Philip
We just came across a case internally where this optimization applied not null to a pointer with an address space other than 0.
Sorry, I reviewed this and really should have thought about that at the time.
What are your thoughts on this? I believe we can only assume nullness on address space 0 and have to leave all others alone.
I don’t think we have an optimization pipeline which will expose this, but lets assume that we have something like:
foo(&g)
...
foo(int *addresspace(1) p) {
if (p != null) { bar(); }
}
then you could imagine us propagating the not null from the call to the callee (if all calls to the callee have not null), and so we have
foo(&g __attribute__((not null)) )
...
foo(int *addresspace(1) __attribute__((not null)) p) {
if (p != null) { bar(); }
}
and then some optimization would constant fold the if to give us
foo(int *addresspace(1) __attribute__((not null)) p) {
bar();
}
which might be incorrect if 0 was a valid address on address space 1.
Cheers,
Pete
> On Jun 16, 2015, at 1:24 PM, Philip Reames <listmail at philipreames.com> wrote:
>
> Author: reames
> Date: Tue Jun 16 15:24:25 2015
> New Revision: 239849
>
> URL: http://llvm.org/viewvc/llvm-project?rev=239849&view=rev
> Log:
> Reapply 239795 - [InstCombine] Propagate non-null facts to call parameters
>
> The original change broke clang side tests. I will be submitting those momentarily. This change includes post commit feedback on the original change from from Pete Cooper.
>
> Original Submission comments:
> If a parameter to a function is known non-null, use the existing parameter attributes to record that fact at the call site. This has no optimization benefit by itself - that I know of - but is an enabling change for http://reviews.llvm.org/D9129.
>
> Differential Revision: http://reviews.llvm.org/D9132
>
>
> Modified:
> llvm/trunk/include/llvm/IR/CallSite.h
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> llvm/trunk/test/CodeGen/NVPTX/intrin-nocapture.ll
> llvm/trunk/test/Transforms/Inline/byval-tail-call.ll
> llvm/trunk/test/Transforms/InstCombine/select.ll
>
> Modified: llvm/trunk/include/llvm/IR/CallSite.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/CallSite.h?rev=239849&r1=239848&r2=239849&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/CallSite.h (original)
> +++ llvm/trunk/include/llvm/IR/CallSite.h Tue Jun 16 15:24:25 2015
> @@ -27,6 +27,7 @@
> #define LLVM_IR_CALLSITE_H
>
> #include "llvm/ADT/PointerIntPair.h"
> +#include "llvm/ADT/iterator_range.h"
> #include "llvm/IR/Attributes.h"
> #include "llvm/IR/CallingConv.h"
> #include "llvm/IR/Instructions.h"
> @@ -150,6 +151,9 @@ public:
> }
>
> IterTy arg_end() const { return (*this)->op_end() - getArgumentEndOffset(); }
> + iterator_range<IterTy> args() const {
> + return iterator_range<IterTy>(arg_begin(), arg_end());
> + }
> bool arg_empty() const { return arg_end() == arg_begin(); }
> unsigned arg_size() const { return unsigned(arg_end() - arg_begin()); }
>
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=239849&r1=239848&r2=239849&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Tue Jun 16 15:24:25 2015
> @@ -1391,11 +1391,29 @@ static IntrinsicInst *FindInitTrampoline
> // visitCallSite - Improvements for call and invoke instructions.
> //
> Instruction *InstCombiner::visitCallSite(CallSite CS) {
> +
> if (isAllocLikeFn(CS.getInstruction(), TLI))
> return visitAllocSite(*CS.getInstruction());
>
> bool Changed = false;
>
> + // Mark any parameters that are known to be non-null with the nonnull
> + // attribute. This is helpful for inlining calls to functions with null
> + // checks on their arguments.
> + unsigned ArgNo = 0;
> + for (Value *V : CS.args()) {
> + if (!CS.paramHasAttr(ArgNo+1, Attribute::NonNull) &&
> + isKnownNonNull(V)) {
> + AttributeSet AS = CS.getAttributes();
> + AS = AS.addAttribute(CS.getInstruction()->getContext(), ArgNo+1,
> + Attribute::NonNull);
> + CS.setAttributes(AS);
> + Changed = true;
> + }
> + ArgNo++;
> + }
> + assert(ArgNo == CS.arg_size() && "sanity check");
> +
> // If the callee is a pointer to a function, attempt to move any casts to the
> // arguments of the call/invoke.
> Value *Callee = CS.getCalledValue();
>
> Modified: llvm/trunk/test/CodeGen/NVPTX/intrin-nocapture.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/NVPTX/intrin-nocapture.ll?rev=239849&r1=239848&r2=239849&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/NVPTX/intrin-nocapture.ll (original)
> +++ llvm/trunk/test/CodeGen/NVPTX/intrin-nocapture.ll Tue Jun 16 15:24:25 2015
> @@ -11,7 +11,7 @@ declare i32 addrspace(1)* @llvm.nvvm.ptr
> ; CHECK: @bar
> define void @bar() {
> %t1 = alloca i32
> -; CHECK: call i32 addrspace(1)* @llvm.nvvm.ptr.gen.to.global.p1i32.p0i32(i32* %t1)
> +; CHECK: call i32 addrspace(1)* @llvm.nvvm.ptr.gen.to.global.p1i32.p0i32(i32* nonnull %t1)
> ; CHECK-NEXT: store i32 10, i32* %t1
> %t2 = call i32 addrspace(1)* @llvm.nvvm.ptr.gen.to.global.p1i32.p0i32(i32* %t1)
> store i32 10, i32* %t1
>
> Modified: llvm/trunk/test/Transforms/Inline/byval-tail-call.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/byval-tail-call.ll?rev=239849&r1=239848&r2=239849&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/Inline/byval-tail-call.ll (original)
> +++ llvm/trunk/test/Transforms/Inline/byval-tail-call.ll Tue Jun 16 15:24:25 2015
> @@ -33,7 +33,7 @@ define void @frob(i32* %x) {
> ; CHECK: %[[POS:.*]] = alloca i32
> ; CHECK: %[[VAL:.*]] = load i32, i32* %x
> ; CHECK: store i32 %[[VAL]], i32* %[[POS]]
> -; CHECK: {{^ *}}call void @ext(i32* %[[POS]]
> +; CHECK: {{^ *}}call void @ext(i32* nonnull %[[POS]]
> ; CHECK: tail call void @ext(i32* null)
> ; CHECK: ret void
> tail call void @qux(i32* byval %x)
>
> Modified: llvm/trunk/test/Transforms/InstCombine/select.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/select.ll?rev=239849&r1=239848&r2=239849&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/select.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/select.ll Tue Jun 16 15:24:25 2015
> @@ -1265,7 +1265,7 @@ define i32 @test77(i1 %flag, i32* %x) {
> ; load does.
> ; CHECK-LABEL: @test77(
> ; CHECK: %[[A:.*]] = alloca i32, align 1
> -; CHECK: call void @scribble_on_i32(i32* %[[A]])
> +; CHECK: call void @scribble_on_i32(i32* nonnull %[[A]])
> ; CHECK: store i32 0, i32* %x
> ; CHECK: %[[P:.*]] = select i1 %flag, i32* %[[A]], i32* %x
> ; CHECK: load i32, i32* %[[P]]
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list