<div dir="ltr">Thanks! Filed as <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D23809&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=8UhviUuJ5CtOKaCUUZRnNw_1UviITzrKvtichJIOX1M&s=Af6JVw4FSqpLtz2K2hFn1eNOju2_FfXlTjiwbqiS4tM&e=">https://llvm.org/bugs/show_bug.cgi?id=23809</a>. </div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 10, 2015 at 5:32 AM, 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: "Jingyue Wu" <<a href="mailto:jingyue@google.com">jingyue@google.com</a>><br>
> To: "LLVM Developers Mailing List" <<a href="mailto:llvmdev@cs.uiuc.edu">llvmdev@cs.uiuc.edu</a>><br>
> Cc: "David Majnemer" <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> Sent: Wednesday, June 10, 2015 1:56:54 AM<br>
> Subject: should InstCombine preserve @llvm.assume?<br>
><br>
><br>
><br>
> Hi,<br>
><br>
><br>
> I have some WIP that leverages @llvm.assume in some optimization<br>
> passes other than InstCombine. However, it doesn't work yet because<br>
> InstCombine removes @llvm.assume calls that are useful for later<br>
> optimizations. For example, given<br>
><br>
><br>
><br>
> define i32 @foo(i32 %a, i32 %b) {<br>
> %sum = add i32 %a, %b<br>
> %1 = icmp sge i32 %sum, 0<br>
> call void @llvm.assume(i1 %1)<br>
> ret i32 %sum<br>
> }<br>
><br>
><br>
> "opt -instcombine" emits<br>
><br>
><br>
><br>
> define i32 @foo(i32 %a, i32 %b) {<br>
> %sum = add i32 %a, %b<br>
> ret i32 %sum<br>
> }<br>
><br>
><br>
> removing the @llvm.assume call so later optimizations won't know sum<br>
> is non-negative any more. (Note that the opt I use here is with<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10283&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=8UhviUuJ5CtOKaCUUZRnNw_1UviITzrKvtichJIOX1M&s=JbBC1Fgt0QcjUhfpTZg_QNy1uZaNCm9MDaSrQ2vgXqw&e=" target="_blank">http://reviews.llvm.org/D10283</a> patched. This patch fixes a bug in<br>
> ValueTracking).<br>
><br>
><br>
> The reasons that InstCombine removes this assume call are:<br>
> 1) SimplifyICmpInst proves %1 is true based on the assume call.<br>
> 2) then, InstCombine (<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_docs_doxygen_html_InstCombineCompares-5F8cpp-5Fsource.html-23l02649&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=8UhviUuJ5CtOKaCUUZRnNw_1UviITzrKvtichJIOX1M&s=m-aK35nn33vH_txUYhbxgCJ-Xq5pE9iLxUnEegErSRE&e=" target="_blank">http://llvm.org/docs/doxygen/html/InstCombineCompares_8cpp_source.html#l02649</a><br>
> ) replaces all uses of %1 with true including the use in the assume<br>
> call.<br>
> 3) Somewhere later, llvm.assume(true) is considered trivially dead<br>
> and thus removed from the IR.<br>
<br>
</div></div>This is a bug; my guess is that the context instruction is not being set correctly in this case. The @llvm.assume should never be used to simplify instructions that are used only by the @llvm.assume. If the context instruction is set correctly, the logic in isValidAssumeForContext should prevent this.<br>
<br>
Feel free to file a bug with this IR and I'll look at it.<br>
<span class=""><br>
><br>
><br>
> Step 2 looks particularly problematic to me. Removing @llvm.assume<br>
> essentially throws away the base of the proof so we won't be able to<br>
> make the same proof any more.<br>
><br>
><br>
><br>
> How can we fix this issue? One heavy-handed approach is, instead of<br>
> RAUW the icmp, we only replace the uses that are not by<br>
> @llvm.assume. But I have two concerns.<br>
><br>
><br>
> 1) what if the icmp is not directly used by an @llvm.assume? e.g., if<br>
> we proved a >= 0 and that condition is used in assume(a >= 0 || b >=<br>
> 0), should we keep (a >= 0) in case later passes use them? If yes,<br>
> we would probably have to recursively traverse def-use chains. If<br>
> no, some assumption is again lost after instcombine.<br>
><br>
><br>
> 2) what if the kept assumes are not leveraged later at all? These<br>
> assumes bump up values' refcounts and could potentially hurt<br>
> optimizations. This looks like a problem for adding @llvm.assume in<br>
> general. Maybe users should be aware of these trade-offs when adding<br>
> __builtin_assumes in their source code.<br>
<br>
</span>Yes, this is the established tradeoff. The LangRef contains a warning about this explicitly:<br>
<br>
"Note that the optimizer might limit the transformations performed on values used by the llvm.assume intrinsic in order to preserve the instructions only used to form the intrinsic’s input argument. This might prove undesirable if the extra information provided by the llvm.assume intrinsic does not cause sufficient overall improvement in code quality. For this reason, llvm.assume should not be used to document basic mathematical invariants that the optimizer can otherwise deduce or facts that are of little use to the optimizer."<br>
<br>
 -Hal<br>
<br>
><br>
><br>
> Thoughts?<br>
><br>
><br>
> Thanks,<br>
> Jingyue<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div>