[PATCH] `nonnull` argument attribute.
Luqman Aden
me+llvm at luqman.ca
Mon May 19 10:53:06 PDT 2014
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>
More information about the llvm-commits
mailing list