<div dir="ltr">My comments were the same as Aaron's =)<div><br></div><div>Other than that, LGTM.<div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 11, 2014 at 9:49 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Jul 11, 2014 at 1:50 AM, <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
> Fixed up the remaining regression tests (now all pass), and added a new test for dereferenceable/nonnull on non-addrspace(0) references.<br>
><br>
> <a href="http://reviews.llvm.org/D4450" target="_blank">http://reviews.llvm.org/D4450</a><br>
><br>
> Files:<br>
> lib/CodeGen/CGCall.cpp<br>
> test/CXX/except/except.spec/p14-ir.cpp<br>
> test/CodeGenCXX/address-space-ref.cpp<br>
> test/CodeGenCXX/blocks-cxx11.cpp<br>
> test/CodeGenCXX/blocks.cpp<br>
> test/CodeGenCXX/catch-undef-behavior.cpp<br>
> test/CodeGenCXX/conditional-gnu-ext.cpp<br>
> test/CodeGenCXX/const-init-cxx11.cpp<br>
> test/CodeGenCXX/constructor-direct-call.cpp<br>
> test/CodeGenCXX/constructor-init.cpp<br>
> test/CodeGenCXX/convert-to-fptr.cpp<br>
> test/CodeGenCXX/copy-assign-synthesis-1.cpp<br>
> test/CodeGenCXX/copy-constructor-elim-2.cpp<br>
> test/CodeGenCXX/copy-constructor-synthesis-2.cpp<br>
> test/CodeGenCXX/copy-constructor-synthesis.cpp<br>
> test/CodeGenCXX/cxx11-initializer-aggregate.cpp<br>
> test/CodeGenCXX/cxx11-thread-local-reference.cpp<br>
> test/CodeGenCXX/decl-ref-init.cpp<br>
> test/CodeGenCXX/default-arg-temps.cpp<br>
> test/CodeGenCXX/derived-to-base-conv.cpp<br>
> test/CodeGenCXX/derived-to-virtual-base-class-calls-final.cpp<br>
> test/CodeGenCXX/dllexport-members.cpp<br>
> test/CodeGenCXX/dllexport.cpp<br>
> test/CodeGenCXX/dllimport-members.cpp<br>
> test/CodeGenCXX/dllimport.cpp<br>
> test/CodeGenCXX/eh.cpp<br>
> test/CodeGenCXX/empty-nontrivially-copyable.cpp<br>
> test/CodeGenCXX/exceptions.cpp<br>
> test/CodeGenCXX/fastcall.cpp<br>
> test/CodeGenCXX/goto.cpp<br>
> test/CodeGenCXX/implicit-copy-assign-operator.cpp<br>
> test/CodeGenCXX/implicit-copy-constructor.cpp<br>
> test/CodeGenCXX/mangle-lambdas.cpp<br>
> test/CodeGenCXX/mangle.cpp<br>
> test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp<br>
> test/CodeGenCXX/microsoft-abi-static-initializers.cpp<br>
> test/CodeGenCXX/nrvo.cpp<br>
> test/CodeGenCXX/pod-member-memcpys.cpp<br>
> test/CodeGenCXX/pointers-to-data-members.cpp<br>
> test/CodeGenCXX/reference-cast.cpp<br>
> test/CodeGenCXX/rvalue-references.cpp<br>
> test/CodeGenCXX/temporaries.cpp<br>
> test/CodeGenCXX/throw-expressions.cpp<br>
> test/CodeGenCXX/volatile.cpp<br>
> test/CodeGenObjC/<a href="http://return-objc-object.mm" target="_blank">return-objc-object.mm</a><br>
> test/CodeGenObjCXX/<a href="http://arc-blocks.mm" target="_blank">arc-blocks.mm</a><br>
> test/CodeGenObjCXX/<a href="http://arc-move.mm" target="_blank">arc-move.mm</a><br>
> test/CodeGenObjCXX/<a href="http://arc-special-member-functions.mm" target="_blank">arc-special-member-functions.mm</a><br>
> test/CodeGenObjCXX/<a href="http://implicit-copy-assign-operator.mm" target="_blank">implicit-copy-assign-operator.mm</a><br>
> test/CodeGenObjCXX/<a href="http://implicit-copy-constructor.mm" target="_blank">implicit-copy-constructor.mm</a><br>
> test/CodeGenObjCXX/<a href="http://lvalue-reference-getter.mm" target="_blank">lvalue-reference-getter.mm</a><br>
> test/CodeGenObjCXX/<a href="http://message-reference.mm" target="_blank">message-reference.mm</a><br>
> test/CodeGenObjCXX/<a href="http://property-dot-reference.mm" target="_blank">property-dot-reference.mm</a><br>
> test/CodeGenObjCXX/<a href="http://property-lvalue-capture.mm" target="_blank">property-lvalue-capture.mm</a><br>
> test/CodeGenObjCXX/<a href="http://property-object-reference-2.mm" target="_blank">property-object-reference-2.mm</a><br>
> test/CodeGenObjCXX/<a href="http://property-objects.mm" target="_blank">property-objects.mm</a><br>
> test/CodeGenObjCXX/<a href="http://property-reference.mm" target="_blank">property-reference.mm</a><br>
> test/Modules/<a href="http://templates.mm" target="_blank">templates.mm</a><br>
<br>
</div></div>I have no real comments on whether this is the correct approach or<br>
not, but I do have a small style nit:<br>
<div class=""><br>
> Index: lib/CodeGen/CGCall.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/CGCall.cpp<br>
> +++ lib/CodeGen/CGCall.cpp<br>
</div>> @@ -1200,8 +1200,14 @@<br>
<div class="">> llvm_unreachable("Invalid ABI kind for return argument");<br>
> }<br>
><br>
> - if (RetTy->isReferenceType())<br>
</div>> - RetAttrs.addAttribute(llvm::Attribute::NonNull);<br>
<div class="">> + if (RetTy->isReferenceType()) {<br>
> + QualType PTy = RetTy->getAs<ReferenceType>()->getPointeeType();<br>
<br>
</div>if (const auto *RefTy = RetTy->getAs<ReferenceType>()) {<br>
QualType Pty = RefTy->getPointeeType();<br>
...<br>
<br>
> + if (!PTy->isIncompleteType() && PTy->isConstantSizeType())<br>
> + RetAttrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)<br>
> + .getQuantity());<br>
> + else if (getContext().getTargetAddressSpace(PTy) == 0)<br>
> + RetAttrs.addAttribute(llvm::Attribute::NonNull);<br>
> + }<br>
><br>
> if (RetAttrs.hasAttributes())<br>
> PAL.push_back(llvm::<br>
> @@ -1291,8 +1297,14 @@<br>
> }<br>
> }<br>
><br>
> - if (ParamType->isReferenceType())<br>
> - Attrs.addAttribute(llvm::Attribute::NonNull);<br>
<div class="">> + if (ParamType->isReferenceType()) {<br>
> + QualType PTy = ParamType->getAs<ReferenceType>()->getPointeeType();<br>
<br>
</div>Same here as above.<br>
<br>
> + if (!PTy->isIncompleteType() && PTy->isConstantSizeType())<br>
> + Attrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)<br>
> + .getQuantity());<br>
> + else if (getContext().getTargetAddressSpace(PTy) == 0)<br>
<div class="HOEnZb"><div class="h5">> + Attrs.addAttribute(llvm::Attribute::NonNull);<br>
> + }<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">~Aaron<br>
</font></span></blockquote></div><br></div></div></div>