[PATCH] `nonnull` argument attribute.

[Luqman Aden]

ping


On Mon, May 12, 2014 at 1:46 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Luqman Aden wrote:
>
>> ping
>>
>
> ​``nonnull``
>         ​ This indicates that the parameter or return pointer is not null.
>         ​ This is an optimization hint to the code generator allowing it
> to possibly
>         ​ omit some null checks. This is not checked or enforced and thus
> the pointer
>         ​ must be guaranteed by the caller to be non-null. This parameter
> may only be
>         ​ applied to pointer types.
>
> This is fine as is, but I'd rather you put the correctness information up
> front, making it:
>
> ``nonnull``
>         ​ This indicates that the parameter or return pointer is not null.
> This attribute may only be applied to pointer typed parameters. This is not
> checked or enforced by LLVM, the caller must ensure that the pointer passed
> in is non-null, or the callee must ensure that the returned pointer is
> non-null.
>
> Something like that.
>
> In ValueTracking.cpp:
>
> // Is it marked not null?
>         ​ if (const Argument *A = dyn_cast<Argument>(V))
>         ​ return A->hasNonNullAttr();
>         ​
>         ​ // A byval or inalloca argument is never null.
>         ​ if (const Argument *A = dyn_cast<Argument>(V))
>         ​ return A->hasByValOrInAllocaAttr();
>         ​
>
> Fine as is, but I'd prefer if you'd fold the two if-statements into one,
> and get:
>
>   if (const Argument *A = dyn_cast<Argument>(V))
>     return A->hasByValOrInAllocaAttr() || A->hasNonNullAttr();
>
> It should only matter in unoptimized builds.
>
> There's more you can do in instcombine, this:
>
>   declare void @callee(i8* nonnull)
>   define void @test() {
>     call @callee(i8* null)
>     ret void
>   }
>
> can be optimized away to unreachable. You don't need to include that in
> this patch.
>
>
> You need to add your attribute to the commented-out block inside the
> definition of LLVMAttribute in include/llvm-c/Core.h .
>
> With that done, please submit.
>
> Nick
>
>  On Tue, Apr 22, 2014 at 1:09 PM, Luqman Aden <me+llvm at luqman.ca
>> <mailto:me+llvm at luqman.ca>> wrote:
>>
>>     ping
>>
>>
>>     On Fri, Apr 18, 2014 at 9:12 PM, Philip Reames
>>     <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>>
>>         Good catch.  This is a real corner case in the attribute
>>         specification.  Possibly we should document the distinction
>>         somewhere?
>>
>>         Which reminds me, we'll need doc changes (LangRef) to describe
>>         the new attribute.
>>
>>         Philip
>>
>>
>>         On 04/17/2014 10:43 PM, Hal Finkel wrote:
>>
>>             ----- Original Message -----
>>
>>                 From: "Nick Lewycky" <nicholas at mxc.ca
>>                 <mailto:nicholas at mxc.ca>>
>>                 To: reviews+D3389+public+ a85d4f162cd7a6e0 at reviews.llvm.
>>                 org
>>                 <mailto:reviews%2BD3389%2Bpublic%2Ba85d4f162cd7a6e0@
>> reviews.llvm.org>
>>                 Cc: llvm-commits at cs.uiuc.edu
>>                 <mailto:llvm-commits at cs.uiuc.edu>
>>
>>                 Sent: Thursday, April 17, 2014 10:52:35 PM
>>                 Subject: Re: [PATCH] `nonnull` argument attribute.
>>
>>                 Your change to lib/IR/Value.cpp is wrong, a nonnull
>>                 pointer is not
>>                 necessarily dereferenceable.
>>
>>                 %a = inttoptr i32 123456 to i32*
>>                 call void @foo(i32* %a nonnull)
>>
>>                 It's nonnull, but not dereferenceable.
>>
>>             I'll add that we really should have a dereferenceable
>>             attribute, IMHO, but as Nick said, it needs to be a separate
>>             from notnull. I think that for dereferenceable we'll want to
>>             have a more involved discussion because we'll need to decide
>>             if it affects derived pointers, should it have a size
>>             parameter, etc.
>>
>>                -Hal
>>
>>                 Nick
>>
>>                 Luqman Aden wrote:
>>
>>                          Added test.
>>
>>                     http://reviews.llvm.org/D3389
>>
>>                     CHANGE SINCE LAST DIFF
>>                     http://reviews.llvm.org/D3389? vs=8552&id=8553#toc
>>                     <http://reviews.llvm.org/D3389?vs=8552&id=8553#toc>
>>
>>                     Files:
>>                          lib/AsmParser/LLToken.h
>>                          lib/AsmParser/LLParser.cpp
>>                          lib/AsmParser/LLLexer.cpp
>>                          lib/Bitcode/Writer/ BitcodeWriter.cpp
>>                          lib/Bitcode/Reader/ BitcodeReader.cpp
>>                          lib/IR/Function.cpp
>>                          lib/IR/Attributes.cpp
>>                          lib/Analysis/ValueTracking.cpp
>>                          test/Bitcode/attributes.ll
>>                          test/Analysis/BasicAA/nonnull. ll
>>                          include/llvm/Bitcode/ LLVMBitCodes.h
>>                          include/llvm/IR/Attributes.h
>>                          include/llvm/IR/Argument.h
>>
>>
>>
>>                     ______________________________ _________________
>>                     llvm-commits mailing list
>>                     llvm-commits at cs.uiuc.edu
>>                     <mailto:llvm-commits at cs.uiuc.edu>
>>                     http://lists.cs.uiuc.edu/
>>                     mailman/listinfo/llvm-commits
>>                     <http://lists.cs.uiuc.edu/
>> mailman/listinfo/llvm-commits>
>>
>>                 ______________________________ _________________
>>                 llvm-commits mailing list
>>                 llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu
>> >
>>                 http://lists.cs.uiuc.edu/ mailman/listinfo/llvm-commits
>>                 <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
>>         ______________________________ _________________
>>         llvm-commits mailing list
>>         llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>         http://lists.cs.uiuc.edu/ mailman/listinfo/llvm-commits
>>         <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140519/3618ad3b/attachment.html>