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