[PATCH] `nonnull` argument attribute.
Philip Reames
listmail at philipreames.com
Mon May 19 13:17:37 PDT 2014
The current state of this patch looks good to me, but I do not have
commit rights. Nick, could you take a look and check this in if you're
okay with it?
Philip
On 05/19/2014 10:53 AM, Luqman Aden wrote:
> ping
>
>
> On Mon, May 12, 2014 at 1:46 PM, Nick Lewycky <nicholas at mxc.ca
> <mailto: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%2Bllvm at luqman.ca>
> <mailto:me+llvm at luqman.ca <mailto:me%2Bllvm 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>
> <mailto: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>
> <mailto:nicholas at mxc.ca <mailto:nicholas at mxc.ca>>>
> To: reviews+D3389+public+
> a85d4f162cd7a6e0 at reviews.llvm.
> org
>
> <mailto:reviews%2BD3389%2Bpublic%2Ba85d4f162cd7a6e0 at reviews.llvm.org
> <mailto:reviews%252BD3389%252Bpublic%252Ba85d4f162cd7a6e0 at reviews.llvm.org>>
> Cc: llvm-commits at cs.uiuc.edu
> <mailto:llvm-commits at cs.uiuc.edu>
> <mailto: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>
> <mailto: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>
> <mailto: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>
> <mailto: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
> 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/5251524b/attachment.html>
More information about the llvm-commits
mailing list