<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 11, 2014 at 4:15 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</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">----- Original Message -----<br>
> From: "David Blaikie" <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>><br>
> To: "Philip Reames" <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>><br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> Sent: Tuesday, November 11, 2014 5:47:28 PM<br>
> Subject: Re: [llvm] r221737 - Canonicalize an assume(load != null) into       !nonnull metadata<br>
><br>
> On Tue, Nov 11, 2014 at 3:33 PM, Philip Reames <<br>
> <a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a> > wrote:<br>
><br>
><br>
> Author: reames<br>
> Date: Tue Nov 11 17:33:19 2014<br>
> New Revision: 221737<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=221737&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=221737&view=rev</a><br>
> Log:<br>
> Canonicalize an assume(load != null) into !nonnull metadata<br>
><br>
> We currently have two ways of informing the optimizer that the result<br>
> of a load is never null: metadata and assume. This change converts<br>
> the second in to the former. This avoids a need to implement<br>
> optimizations using both forms.<br>
><br>
> We should probably extend this basic idea to metadata of other forms;<br>
> in particular, range metadata. We view is that assumes should be<br>
> considered a "last resort" for when there isn't a more canonical way<br>
> to represent something.<br>
><br>
><br>
> (naively) seems almost a pity, really - if we have the general tool,<br>
> why not drop the special cases & just use the general one? For<br>
> things already using the special cases, they can continue to look<br>
> for the very specific canonicalized case of the general form with<br>
> relatively low cost, I would imagine - but I know very little about<br>
> all of this. Just a thought.<br>
><br>
<br>
</div></div>I share your desire for making use of a single general-purpose tool, but in this case, using @llvm.assume as a much higher cost compared to the metadata (both in its representational size and in its ability to block potentially-useful optimizations),</blockquote><div><br></div><div>How does it block optimizations? That's sort of saddening.</div><div><br></div><div>-- Sean Silva</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> so I think it makes sense to canonicalize on the metadata when available.<br>
<br>
 -Hal<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
><br>
> Reviewed by: Hal<br>
> Differential Revision: <a href="http://reviews.llvm.org/D5951" target="_blank">http://reviews.llvm.org/D5951</a><br>
><br>
><br>
> Modified:<br>
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> llvm/trunk/test/Transforms/InstCombine/assume.ll<br>
><br>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=221737&r1=221736&r2=221737&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=221737&r1=221736&r2=221737&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> (original)<br>
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Tue<br>
> Nov 11 17:33:19 2014<br>
> @@ -1102,6 +1102,26 @@ Instruction *InstCombiner::visitCallInst<br>
> return EraseInstFromFunction(*II);<br>
> }<br>
><br>
> + // assume( (load addr) != null ) -> add 'nonnull' metadata to load<br>
> + // (if assume is valid at the load)<br>
> + if (ICmpInst* ICmp = dyn_cast<ICmpInst>(IIOperand)) {<br>
> + Value *LHS = ICmp->getOperand(0);<br>
> + Value *RHS = ICmp->getOperand(1);<br>
> + if (ICmpInst::ICMP_NE == ICmp->getPredicate() &&<br>
> + isa<LoadInst>(LHS) &&<br>
> + isa<Constant>(RHS) &&<br>
> + RHS->getType()->isPointerTy() &&<br>
> + cast<Constant>(RHS)->isNullValue()) {<br>
> + LoadInst* LI = cast<LoadInst>(LHS);<br>
> + if (isValidAssumeForContext(II, LI, DL, DT)) {<br>
> + MDNode* MD = MDNode::get(II->getContext(), ArrayRef<Value*>());<br>
> + LI->setMetadata(LLVMContext::MD_nonnull, MD);<br>
> + return EraseInstFromFunction(*II);<br>
> + }<br>
> + }<br>
> + // TODO: apply nonnull return attributes to calls and invokes<br>
> + // TODO: apply range metadata for range check patterns?<br>
> + }<br>
> // If there is a dominating assume with the same condition as this<br>
> one,<br>
> // then this one is redundant, and should be removed.<br>
> APInt KnownZero(1, 0), KnownOne(1, 0);<br>
><br>
> Modified: llvm/trunk/test/Transforms/InstCombine/assume.ll<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/assume.ll?rev=221737&r1=221736&r2=221737&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/assume.ll?rev=221737&r1=221736&r2=221737&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/InstCombine/assume.ll (original)<br>
> +++ llvm/trunk/test/Transforms/InstCombine/assume.ll Tue Nov 11<br>
> 17:33:19 2014<br>
> @@ -186,6 +186,80 @@ entry:<br>
> ; CHECK: ret i32 0<br>
> }<br>
><br>
> +declare void @escape(i32* %a)<br>
> +<br>
> +; Do we canonicalize a nonnull assumption on a load into<br>
> +; metadata form?<br>
> +define i1 @nonnull1(i32** %a) {<br>
> +entry:<br>
> + %load = load i32** %a<br>
> + %cmp = icmp ne i32* %load, null<br>
> + tail call void @llvm.assume(i1 %cmp)<br>
> + tail call void @escape(i32* %load)<br>
> + %rval = icmp eq i32* %load, null<br>
> + ret i1 %rval<br>
> +<br>
> +; CHECK-LABEL: @nonnull1<br>
> +; CHECK: !nonnull<br>
> +; CHECK-NOT: call void @llvm.assume<br>
> +; CHECK: ret i1 false<br>
> +}<br>
> +<br>
> +; Make sure the above canonicalization applies only<br>
> +; to pointer types. Doing otherwise would be illegal.<br>
> +define i1 @nonnull2(i32* %a) {<br>
> +entry:<br>
> + %load = load i32* %a<br>
> + %cmp = icmp ne i32 %load, 0<br>
> + tail call void @llvm.assume(i1 %cmp)<br>
> + %rval = icmp eq i32 %load, 0<br>
> + ret i1 %rval<br>
> +<br>
> +; CHECK-LABEL: @nonnull2<br>
> +; CHECK-NOT: !nonnull<br>
> +; CHECK: call void @llvm.assume<br>
> +}<br>
> +<br>
> +; Make sure the above canonicalization does not trigger<br>
> +; if the assume is control dependent on something else<br>
> +define i1 @nonnull3(i32** %a, i1 %control) {<br>
> +entry:<br>
> + %load = load i32** %a<br>
> + %cmp = icmp ne i32* %load, null<br>
> + br i1 %control, label %taken, label %not_taken<br>
> +taken:<br>
> + tail call void @llvm.assume(i1 %cmp)<br>
> + %rval = icmp eq i32* %load, null<br>
> + ret i1 %rval<br>
> +not_taken:<br>
> + ret i1 true<br>
> +<br>
> +; CHECK-LABEL: @nonnull3<br>
> +; CHECK-NOT: !nonnull<br>
> +; CHECK: call void @llvm.assume<br>
> +}<br>
> +<br>
> +; Make sure the above canonicalization does not trigger<br>
> +; if the path from the load to the assume is potentially<br>
> +; interrupted by an exception being thrown<br>
> +define i1 @nonnull4(i32** %a) {<br>
> +entry:<br>
> + %load = load i32** %a<br>
> + ;; This call may throw!<br>
> + tail call void @escape(i32* %load)<br>
> + %cmp = icmp ne i32* %load, null<br>
> + tail call void @llvm.assume(i1 %cmp)<br>
> + %rval = icmp eq i32* %load, null<br>
> + ret i1 %rval<br>
> +<br>
> +; CHECK-LABEL: @nonnull4<br>
> +; CHECK-NOT: !nonnull<br>
> +; CHECK: call void @llvm.assume<br>
> +}<br>
> +<br>
> +<br>
> +<br>
> +<br>
> attributes #0 = { nounwind uwtable }<br>
> attributes #1 = { nounwind }<br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>