[llvm] r247356 - [InstCombineCalls] Use isKnownNonNullAt() to check nullness of passing arguments at callsite

[Sanjoy Das via llvm-commits]

Chen Li wrote:
>> On Sep 10, 2015, at 6:41 PM, Sanjoy Das<sanjoy at playingwithpointers.com>  wrote:
>>
>> Yeah, I think this is marking non pointer arguments as nonnull.
>>
>> This was probably an existing issue that did not happen in practice
>> because `isKnownNonNull` does not return true for non pointer values.
>> `isKnownNonNullAt` is smarter and will return true for a provably
>> non-zero integer.
>>
>
> Does it make sense to say a non pointer type value nonnull? Or the nonnull attribute should be extended to integer type values as well. Otherwise it doesn't seem to be consistent.
>

This certainly is somewhat confusing, but within the codebase, Null is 
sometimes used used interchangeably with 0.  That is distinct from the 
`null` IR level value.

For instance, Constant *Constant::getNullValue(Type *Ty) will return you 
0 if Ty is an integer type, a vector of zeroes if Ty is a vector type etc.

-- Sanjoy

> Chen
>
>> -- Sanjoy
>>
>> On Thu, Sep 10, 2015 at 6:38 PM, Mehdi Amini via llvm-commits
>> <llvm-commits at lists.llvm.org>  wrote:
>>> Hi,
>>>
>>> I reverted in r247371 because it broke llvm/test/Transforms/InstCombine/pr8547.ll:
>>>
>>> --
>>> Exit Code: 2
>>>
>>> Command Output (stderr):
>>> --
>>> Wrong types for attribute: byval inalloca nest noalias nocapture nonnull readnone readonly sret dereferenceable(1) dereferenceable_or_null(1)
>>>   %call = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @.str, i64 0, i64 0), i32 nonnull %conv2) #0
>>> LLVM ERROR: Broken function found, compilation aborted!
>>> FileCheck error: '-' is empty.
>>>
>>> --
>>>
>>> See for instance: http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux/builds/28054/steps/test_llvm/logs/stdio
>>>
>>> Best,
>>>
>>> —
>>> Mehdi
>>>
>>>> On Sep 10, 2015, at 4:04 PM, Chen Li via llvm-commits<llvm-commits at lists.llvm.org>  wrote:
>>>>
>>>> Author: chenli
>>>> Date: Thu Sep 10 18:04:49 2015
>>>> New Revision: 247356
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=247356&view=rev
>>>> Log:
>>>> [InstCombineCalls] Use isKnownNonNullAt() to check nullness of passing arguments at callsite
>>>>
>>>> Summary: This patch replaces isKnownNonNull() with isKnownNonNullAt() when checking nullness of passing arguments at callsite. In this way it can handle cases where the argument does not have nonnull attribute but has a dominating null check from the CFG.
>>>>
>>>> Reviewers: reames
>>>>
>>>> Subscribers: llvm-commits
>>>>
>>>> Differential Revision: http://reviews.llvm.org/D12779
>>>>
>>>> Added:
>>>>    llvm/trunk/test/Transforms/InstCombine/call_nonnull_arg.ll
>>>> Modified:
>>>>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=247356&r1=247355&r2=247356&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Thu Sep 10 18:04:49 2015
>>>> @@ -1537,7 +1537,7 @@ Instruction *InstCombiner::visitCallSite
>>>>   unsigned ArgNo = 0;
>>>>   for (Value *V : CS.args()) {
>>>>     if (!CS.paramHasAttr(ArgNo+1, Attribute::NonNull)&&
>>>> -        isKnownNonNull(V)) {
>>>> +        isKnownNonNullAt(V, CS.getInstruction(), DT, TLI)) {
>>>>       AttributeSet AS = CS.getAttributes();
>>>>       AS = AS.addAttribute(CS.getInstruction()->getContext(), ArgNo+1,
>>>>                            Attribute::NonNull);
>>>>
>>>> Added: llvm/trunk/test/Transforms/InstCombine/call_nonnull_arg.ll
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/call_nonnull_arg.ll?rev=247356&view=auto
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/InstCombine/call_nonnull_arg.ll (added)
>>>> +++ llvm/trunk/test/Transforms/InstCombine/call_nonnull_arg.ll Thu Sep 10 18:04:49 2015
>>>> @@ -0,0 +1,17 @@
>>>> +; RUN: opt<  %s -instcombine -S | FileCheck %s
>>>> +
>>>> +; InstCombine should mark null-checked argument as nonnull at callsite
>>>> +declare void @dummy(i32*)
>>>> +
>>>> +define void @test(i32* %a) {
>>>> +; CHECK-LABEL: @test
>>>> +; CHECK: call void @dummy(i32* nonnull %a)
>>>> +entry:
>>>> +  %cond = icmp eq i32* %a, null
>>>> +  br i1 %cond, label %is_null, label %not_null
>>>> +not_null:
>>>> +  call void @dummy(i32* %a)
>>>> +  ret void
>>>> +is_null:
>>>> +  unreachable
>>>> +}
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>> --
>> Sanjoy Das
>> http://playingwithpointers.com