[PATCH] `nonnull` argument attribute.

[Nick Lewycky]

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 at 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>
>
>
>