<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Strange, I remember the whole llvm testsuite working when I was working on the patch (2 months ago). I'll see if I can reproduce it.</div><div class=""><br class=""></div><div class="">- Matthias</div><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 4, 2015, at 6:50 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" class="">james@jamesmolloy.co.uk</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Matthias,<br class=""><br class="">Sorry, but I've just bisected a failure in 403.gcc from SPEC2006 to this commit too.<div class=""><br class=""></div><div class="">It's a fairly large test, so getting a reduced testcase for you might be difficult. Do you have access to SPEC2006?</div><div class=""><br class=""></div><div class="">Cheers,</div><div class=""><br class=""></div><div class="">James</div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, 3 Jun 2015 at 19:27 James Molloy <<a href="mailto:james@jamesmolloy.co.uk" class="">james@jamesmolloy.co.uk</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="">Hi Matthias,<br class=""><br class=""><div class="">Thanks for the clarification - I'll take a further look.</div><div class=""><br class=""></div><div class="">Cheers,</div><div class=""><br class=""></div><div class="">James</div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, 3 Jun 2015 at 17:21 Matthias Braun <<a href="mailto:matze@braunis.de" target="_blank" class="">matze@braunis.de</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">I reported my measurements in one of the early comments of the review thread: I only had slight improvements between 0.3 and 1.7% and two 0.5% regressions.</div><div class=""><br class=""></div><div class="">However this commit does not introduce more select instructions. The idea behind it is that for this code:</div><div class=""><div class=""><br class=""></div><div class="">long foo(long a, long b, long v1, long v2) {</div><div class=""> if (a >= v1 && a < v2)</div><div class=""> return b;</div><div class=""> return 0;</div><div class="">}</div></div><div class=""><br class=""></div><div class="">we used to generate:</div><div class=""><br class=""></div><div class=""><div class=""><span style="white-space:pre-wrap" class=""> </span>cmp<span style="white-space:pre-wrap" class=""> </span> x0, x2</div><div class=""><span style="white-space:pre-wrap" class=""> </span>cset<span style="white-space:pre-wrap" class=""> </span> w8, ge</div><div class=""><span style="white-space:pre-wrap" class=""> </span>cmp<span style="white-space:pre-wrap" class=""> </span> x0, x3</div><div class=""><span style="white-space:pre-wrap" class=""> </span>cset<span style="white-space:pre-wrap" class=""> </span> w9, lt</div><div class=""><span style="white-space:pre-wrap" class=""> </span>tst<span style="white-space:pre-wrap" class=""> </span> w8, w9</div><div class=""><span style="white-space:pre-wrap" class=""> </span>csel<span style="white-space:pre-wrap" class=""> </span>x0, x1, xzr, ne</div><div class=""><span style="white-space:pre-wrap" class=""> </span>ret</div></div><div class=""><br class=""></div><div class="">in another commit I changed the generic backend to use 2 selects instead:</div><div class=""><br class=""></div><div class=""><div class=""><span style="white-space:pre-wrap" class=""> </span>cmp<span style="white-space:pre-wrap" class=""> </span> x0, x2</div><div class=""><span style="white-space:pre-wrap" class=""> </span>csel<span style="white-space:pre-wrap" class=""> </span>x8, x1, xzr, ge</div><div class=""><span style="white-space:pre-wrap" class=""> </span>cmp<span style="white-space:pre-wrap" class=""> </span> x0, x3</div><div class=""><span style="white-space:pre-wrap" class=""> </span>csel<span style="white-space:pre-wrap" class=""> </span>x0, x8, xzr, lt</div><div class=""><span style="white-space:pre-wrap" class=""> </span>ret</div></div><div class=""><br class=""></div><div class="">in this commit I turned of the two select variant (by overriding shouldNormalizeToSelect()) so the improved cmp/ccmp matching can generate:</div><div class=""><br class=""></div><div class=""><span style="white-space:pre-wrap" class=""> </span>cmp<span style="white-space:pre-wrap" class=""> </span> x0, x2</div><div class=""><div class=""><span style="white-space:pre-wrap" class=""> </span>ccmp<span style="white-space:pre-wrap" class=""> </span>x0, x3, #0, ge</div><div class=""><span style="white-space:pre-wrap" class=""> </span>csel<span style="white-space:pre-wrap" class=""> </span>x0, x1, xzr, lt</div><div class=""><span style="white-space:pre-wrap" class=""> </span>ret</div></div><div class=""><br class=""></div><div class="">In any way the number of select does not increase, if anything it should decrease because we are not normalizing towards the two select sequence anymore. The only thing I can think of that may lead to worse code is that the third sequence requires the cmp/ccmp to be scheduled pretty close to each other to not loose the flags, this could lead to increased register pressure as operand computations now happen before that, while for the 2 select version you could schedule the computations in between the two selects.</div><div class=""><br class=""></div><div class="">Anyway I think this would need a more in-depth analysis to really understand what is going on in your benchmark.</div></div><div style="word-wrap:break-word" class=""><div class=""><br class=""></div><div class="">- Matthias</div></div><div style="word-wrap:break-word" class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Jun 3, 2015, at 6:59 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank" class="">james@jamesmolloy.co.uk</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class="">Hi Matthias,<br class=""><br class="">This actually caused a 10% regression in one of our tests on Cortex-A57 (but a 4% improvement on Cortex-A53). I think this is to do with selects being expensive on heavily out of order architectures. Did you notice any regressions on Typhoon/Cyclone?<div class=""><br class=""></div><div class="">It might be useful to have a heuristic determining if this is beneficial or not - if conversion in CGP already has this "isPredictableSelectExpensive()" hook - perhaps a similar one might be useful here?</div><div class=""><br class=""></div><div class="">Cheers,</div><div class=""><br class=""></div><div class="">James</div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, 1 Jun 2015 at 23:39 Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">REPOSITORY<br class="">
rL LLVM<br class="">
<br class="">
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D8232&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Gdqbblg081QXZ4Qg7OhixsHvneH_kDHGwUBZ8ZKdOTc&s=vje1lx_YnX_sc-CYJcXvlF3-Yt-dC4mR2IaDt_3uA4w&e=" target="_blank" class="">http://reviews.llvm.org/D8232</a><br class="">
<br class="">
Files:<br class="">
llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br class="">
llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h<br class="">
llvm/trunk/lib/Target/AArch64/AArch64InstrFormats.td<br class="">
llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td<br class="">
llvm/trunk/test/CodeGen/AArch64/arm64-ccmp.ll<br class="">
<br class="">
EMAIL PREFERENCES<br class="">
<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=Gdqbblg081QXZ4Qg7OhixsHvneH_kDHGwUBZ8ZKdOTc&s=OtEecvfwv8xDM2A9oXbehLujvOjjYcttTI8WpYPmLzE&e=" target="_blank" class="">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="">llvm-commits@cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="">llvm-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class=""></div></blockquote></div><br class=""></div></blockquote></div></blockquote></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></body></html>