<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 25, 2015 at 11:29 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Thu, Jun 25, 2015 at 11:22 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"><span>----- Original Message -----<br>
> From: "Jingyue Wu" <<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a>><br>
> To: "David Majnemer" <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>><br>
> Cc: <a href="mailto:reviews%2BD10695%2Bpublic%2B1cd92e3c8232918a@reviews.llvm.org" target="_blank">reviews+D10695+public+1cd92e3c8232918a@reviews.llvm.org</a>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, "henry hu sh"<br>
> <<a href="mailto:henry.hu.sh@gmail.com" target="_blank">henry.hu.sh@gmail.com</a>>, "LLVM Commits" <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>>, "Xuetian Weng" <<a href="mailto:xweng@google.com" target="_blank">xweng@google.com</a>>, "Bjarke<br>
> Roune" <<a href="mailto:broune@google.com" target="_blank">broune@google.com</a>>, "Mark Heffernan" <<a href="mailto:meheff@google.com" target="_blank">meheff@google.com</a>>, "Eli Bendersky" <<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>><br>
> Sent: Thursday, June 25, 2015 12:56:13 PM<br>
> Subject: Re: [PATCH] [InstCombine] call SimplifyICmpInst with correct context<br>
><br>
><br>
> Agreed. Let me see how the correctness issue can be fixed.<br>
><br>
<br>
</span>I'm missing something. What's the correctness issue?<br></blockquote><div><br></div></span><div>My concern is that there is no way to communicate that no assume context is appropriate for analyzing an instruction.</div></div></div></div></blockquote><div><br></div><div>However, maybe this isn't problematic.  I couldn't come up with a plausible scenario where we'd want to run without a context because doing so might result in miscompiles and the only harm in inferring a context is that we might nuke an assume earlier than we might've.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
 -Hal<br>
<div><div><br>
><br>
> On Wed, Jun 24, 2015 at 10:23 PM, David Majnemer <<br>
> <a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a> > wrote:<br>
><br>
><br>
> On Wed, Jun 24, 2015 at 10:00 PM, Jingyue Wu < <a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a> ><br>
> wrote:<br>
><br>
><br>
> And just to be clear, I agree with you that it's a potential issue<br>
> that `ComputeSignBit` is unable to say "I don't have a context". But<br>
> my concern is, even if we fix that issue, not passing the context<br>
> instruction to `SimplifyICmpInst` at all in `InstCombine` may<br>
> overkill some useful optimizations.<br>
><br>
><br>
><br>
> I see these as two separable issues: a correctness issue and a<br>
> performance issue. My thinking was that we should solve the<br>
> correctness issue by being conservative at the API boundary and<br>
> regain lost performance by providing the context instruction. My<br>
> fear is not that your change to SimplifyICmpInst is incorrect but<br>
> that all the other callers of the various SimplifyXYZInst API have<br>
> latent bugs because they aren't passing in a context.<br>
><br>
><br>
><br>
> Btw, I noticed that `SimplifyDemandedBits` indeed pass `UserI` as the<br>
> context instruction (<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_docs_doxygen_html_InstCombineSimplifyDemanded-5F8cpp-5Fsource.html-23l00075&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=kGx525OLxHx2tGzjizdZ8KNmg6rcNXUFdlX-Coa7LY0&s=EUB_knFarPmbmyxPbB3yhbvuj4N_HTNlmHeCSv2cg2c&e=" rel="noreferrer" target="_blank">http://llvm.org/docs/doxygen/html/InstCombineSimplifyDemanded_8cpp_source.html#l00075</a><br>
> ), which seems aligned with the approach in my patch.<br>
><br>
><br>
><br>
><br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10695&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=kGx525OLxHx2tGzjizdZ8KNmg6rcNXUFdlX-Coa7LY0&s=l-Bfx3PHaKeVW6AOK0gc_EGPYoISKDkFb8W20l3gnLU&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10695</a><br>
><br>
> EMAIL PREFERENCES<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=kGx525OLxHx2tGzjizdZ8KNmg6rcNXUFdlX-Coa7LY0&s=c_G2yAu9D-oNMJcBvZwhJXakWnzxclTPpSf5_163lAc&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
><br>
><br>
><br>
><br>
><br>
<br>
</div></div><span><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>