[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 23:09:50 PDT 2015


> On Aug 25, 2015, at 10:55 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> 
>> On Aug 25, 2015, at 11:09 AM, Pete Cooper via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> 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.
> 
> What is specific to address space 0 compared to the others?
I believe we’ve defined the semantics of address space 0 in terms of whether null is legal or not, but we can’t do so for other address spaces (which have target specific semantics the optimizer is unaware of).  For example in code, see Local.cpp (notice the address space check only for 0):

      // Store to undef and store to null are undefined and used to signal that
      // they should be changed to unreachable by passes that can't modify the
      // CFG.
      if (StoreInst *SI = dyn_cast<StoreInst>(BBI)) {
        // Don't touch volatile stores.
        if (SI->isVolatile()) continue;

        Value *Ptr = SI->getOperand(1);

        if (isa<UndefValue>(Ptr) ||
            (isa<ConstantPointerNull>(Ptr) &&
             SI->getPointerAddressSpace() == 0)) {
          changeToUnreachable(SI, true);
          Changed = true;
          break;
        }
      }


> (I looked up Lang. Ref.  but it is not clear to me)
I just looked too.  Only thing i saw was "	A null pointer in the default address-space is associated with no address.”.  If that’s the relevant part of the docs then its pretty subtle.  You really have to know what you’re trying to look for.

Cheers,
Pete
> 
>> Mehdi
> 
> 
>> 
>> 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: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D239849-26view-3Drev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=PmDOOuAb7S2a3-cG7ucCZ6RdvHNP95AkXZMg5Uxvbv0&e= 
>>> 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 https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9129&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=U2OskthOF-FT8CIuQ-hK-zi1ywpdTwMOYsYxivU-gxI&e= .
>>> 
>>> Differential Revision: https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9132&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=Hkc1Cx1ueqtmbv1v4Z0GDJPccN8xovcX8neQRewSypk&e= 
>>> 
>>> 
>>> 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: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_IR_CallSite.h-3Frev-3D239849-26r1-3D239848-26r2-3D239849-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=kuIaqu0XVNbHEHlcla4h3-7GEivp4UceaFizhVKg_wE&e= 
>>> ==============================================================================
>>> --- 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: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_InstCombine_InstCombineCalls.cpp-3Frev-3D239849-26r1-3D239848-26r2-3D239849-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=8nP1Zo7A8ROnJeDH_w11peVmjUWp08JQRxMcTAmLNsI&e= 
>>> ==============================================================================
>>> --- 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: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_CodeGen_NVPTX_intrin-2Dnocapture.ll-3Frev-3D239849-26r1-3D239848-26r2-3D239849-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=rx8-qj6V4E3i_zgO17T-q1BsUj_tE1S2NppUXSzuaYo&e= 
>>> ==============================================================================
>>> --- 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: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Transforms_Inline_byval-2Dtail-2Dcall.ll-3Frev-3D239849-26r1-3D239848-26r2-3D239849-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=luSZMoJbkTVgcjpOhLqUlDhfbqdztM0mvgMRpawv0lM&e= 
>>> ==============================================================================
>>> --- 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: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Transforms_InstCombine_select.ll-3Frev-3D239849-26r1-3D239848-26r2-3D239849-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=j2RgRQctbtsxF7v5J-HI67wnzQmi6bWfqfxpTd3Ga_M&e= 
>>> ==============================================================================
>>> --- 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
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.cs.uiuc.edu_mailman_listinfo_llvm-2Dcommits&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=unRV5vbavDjvSnXLT14v917tW5C_HnZlrkeF30a_8eY&e= 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=v-ruWq0KCv2O3thJZiK6naxuXK8mQHZUmGq5FBtAmZ4&m=mnvTS572oGZ8pmJUCKsKHIYBcYjRNRFGq-D2DMG_FzM&s=h_wJzvRdrgspfFd7c4_qWF3nuZhfd4uEBfY2L61s-AE&e= 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150825/9fe52766/attachment.html>


More information about the llvm-commits mailing list