<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi James,</div><div class=""><br class=""></div><div class="">from a birds view this optimization needs more work, quite possibly more so in other passes rather than the algorithm itself. In our environment we haven’t seen *any* gains from it. Could you and/or Hao take this on after the code is disabled/backed out?</div><div class=""><br class=""></div><div class="">The twoFish example is a double whammo with code size up *and* performance down: the size of the assembly files differs by 100 lines (~10%), so there must be more to the different than just one simple combiner pattern fix. One issue I spotted quickly are the store pairs (rather than two str) when the optimization is off.</div><div class=""><br class=""></div>Similarly, for test/CodeGen/AArch64/arm64-cse.ll, the differences are:<div class=""><br class=""></div><div class="">< <span style="font-family: Menlo;" class="">-aarch64-gep-opt=false</span> </div><div class="">> <span style="font-family: Menlo;" class="">-aarch64-gep-opt=true </span></div><div class=""><div class=""><div class="" style="margin: 0px; font-family: Menlo;">22,23c22,23</div><div class="" style="margin: 0px; font-family: Menlo;">< <span class="Apple-tab-span" style="white-space: pre;"> </span>ldr<span class="Apple-tab-span" style="white-space: pre;"> </span> w8, [x1]</div><div class="" style="margin: 0px; font-family: Menlo;">< <span class="Apple-tab-span" style="white-space: pre;"> </span>subs<span class="Apple-tab-span" style="white-space: pre;"> </span>w8, w8, #1 ; =1</div><div class="" style="margin: 0px; font-family: Menlo;">---</div><div class="" style="margin: 0px; font-family: Menlo;">> <span class="Apple-tab-span" style="white-space: pre;"> </span>ldrsw<span class="Apple-tab-span" style="white-space: pre;"> </span> x8, [x1]</div><div class="" style="margin: 0px; font-family: Menlo;">> <span class="Apple-tab-span" style="white-space: pre;"> </span>cmp<span class="Apple-tab-span" style="white-space: pre;"> </span> w8, #1 ; =1</div><div class="" style="margin: 0px; font-family: Menlo;">26,27c26,29</div><div class="" style="margin: 0px; font-family: Menlo;">< <span class="Apple-tab-span" style="white-space: pre;"> </span>str<span class="Apple-tab-span" style="white-space: pre;"> </span> w8, [x1]</div><div class="" style="margin: 0px; font-family: Menlo;">< <span class="Apple-tab-span" style="white-space: pre;"> </span>add<span class="Apple-tab-span" style="white-space: pre;"> </span>x0, x0, w8, sxtw</div><div class="" style="margin: 0px; font-family: Menlo;">---</div><div class="" style="margin: 0px; font-family: Menlo;">> <span class="Apple-tab-span" style="white-space: pre;"> </span>sub<span class="Apple-tab-span" style="white-space: pre;"> </span>w9, w8, #1 ; =1</div><div class="" style="margin: 0px; font-family: Menlo;">> <span class="Apple-tab-span" style="white-space: pre;"> </span>add<span class="Apple-tab-span" style="white-space: pre;"> </span> x8, x0, x8</div><div class="" style="margin: 0px; font-family: Menlo;">> <span class="Apple-tab-span" style="white-space: pre;"> </span>str<span class="Apple-tab-span" style="white-space: pre;"> </span> w9, [x1]</div><div class="" style="margin: 0px; font-family: Menlo;">> <span class="Apple-tab-span" style="white-space: pre;"> </span>sub<span class="Apple-tab-span" style="white-space: pre;"> </span>x0, x8, #1 ; =1</div><div class=""><br class=""></div><div class="">FWIW, the other test case that uses the flag to disable the optimization looks OK. It is not clear to me why the flag was needed.</div><div class=""><br class=""></div><div class="">Thanks</div><div class="">Gerolf</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div></div><div class=""><div><blockquote type="cite" class=""><div class="">On Apr 20, 2015, at 1:54 PM, 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 Michael,<br class=""><br class="">From a bleary-eyed look at those dumps, it looks to me as if fewer addressing modes are being matched. For example I'm seeing stuff like this:<div class=""><br class=""></div><div class="">-00000001000314b8<span class="Apple-tab-span" style="white-space:pre"> </span>ubfm<span class="Apple-tab-span" style="white-space:pre"> </span>x1, x2, #1, #8</div><div class="">-00000001000314bc<span class="Apple-tab-span" style="white-space:pre"> </span>ldr<span class="Apple-tab-span" style="white-space:pre"> </span>w1, [x12, x1, lsl #2]</div><div class="">+<font color="#3c80f6" class="">0000000100031be4<span class="Apple-tab-span" style="white-space:pre"> </span>add<span class="Apple-tab-span" style="white-space:pre"> </span>x15, x11, w15, uxtb #2<br class=""></font></div><div class=""><font color="#3c80f6" class="">+0000000100031be8<span class="Apple-tab-span" style="white-space:pre"> </span>ldr<span class="Apple-tab-span" style="white-space:pre"> </span>w15, [x15, #2120]</font></div><div class=""><font color="#3c80f6" class=""><br class=""></font></div><div class=""><font color="#3c80f6" class="">So, is this just pattern permutation - addressing modes aren't being matched as well as they can because the DAG has been permuted? (should be easy to fix if so). Or is it due to information loss somewhere along the line? (I seem to remember that the gep matching pass will produce some inttoptr/ptrtoint pairs sometimes).</font></div><div class=""><font color="#3c80f6" class=""><br class=""></font></div><div class=""><font color="#3c80f6" class="">My main concern with disabling it is the likelihood that it'll be left unfixed, and bitrot. Do you intend to look at it after disabling it? I'd be happier if I knew it was on someone's backlog.</font></div><div class=""><font color="#3c80f6" class=""><br class=""></font></div><div class=""><font color="#3c80f6" class="">Adding Hao, as he wrote this and hopefully can have more insight!</font></div><div class=""><font color="#3c80f6" class=""><br class=""></font></div><div class=""><font color="#3c80f6" class="">Cheers,</font></div><div class=""><font color="#3c80f6" class=""><br class=""></font></div><div class=""><font color="#3c80f6" class="">James</font></div><br class=""><div class="gmail_quote">On Mon, 20 Apr 2015 at 21:36 Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com" class="">mzolotukhin@apple.com</a>> wrote:<br class=""><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="">Hi James,<div class=""><br class=""></div><div class="">I’m attaching two disasm files for the TwoFish hot function, the first one (slow.s) is obtained by compiler with default options, the second one (fast.s) - with aarch64-gep-opt=false.</div><div class="">It’s a pretty big function, and for me it’s not obvious what piece of code is responsible for the total performance. However, slow.s is 100 lines longer, and that’s corresponds to the performance I observe (for slow.s geekbench score of the Twofish is 733, for fast.s - 951, higher is better).</div><div class=""><br class=""></div><div class="">The tests arm64-cse.ll and arm64-addr-mode-folding.ll also might give some ideas what’s wrong - and we need to fix them, or properly explain why it’s ok to disable them (please note that they were written exactly to detect issues that had been fixed once).</div><div class=""><br class=""></div><div class="">I totally support that we need to understand what’s wrong, and what I suggest is a temporal disabling until it’s done.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Michael</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""></div></div><div style="word-wrap:break-word" class=""><div class=""></div></div><div style="word-wrap:break-word" class=""><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Apr 18, 2015, at 9:23 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank" class="">james@jamesmolloy.co.uk</a>> wrote:</div><br class=""><div class="">Hi Michael,<br class=""><br class="">I'm not too keen on disabling this without understanding what it's doing wrong. It seems like a large hammer. <br class=""><br class="">If this is the pass I'm thinking of, it was introduced by Hao to help remove many redundant multiplies formed by complex GEPs, and did manifest in spec, gobmk was the main culprit iirc. <br class=""><br class="">It will also affect in order cores more than out of order ones- would it be ok to understand whether the regression is a deficiency in the pass easily fixed or if it is something more fundamental?<br class=""><br class="">James<br class=""><div class="gmail_quote">On Fri, 17 Apr 2015 at 19:48, Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com" target="_blank" class="">mzolotukhin@apple.com</a>> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br class="">
<br class="">
Recently I’ve found that LLVM regressed on a TwoFish test (from geekbench) when comparing relatively new compiler and a year-old one. The performance regression is pretty big - about 20%, so I decided to investigate it further and understand what change exposed that. Triaging led me to r222328 and r222331, which introduced a new ARM64-specific pass for hoisting constant offsets from addresses. It claimed some gains on SPECS, but when I turned it off, I saw no regressions there (I checked SPEC2006/ref-input set, built with PGO and LTO).<br class="">
<br class="">
Moreover, when the patch was committed, it effectively disabled two existing regression tests, which is not good I think.<br class="">
<br class="">
Is it ok to disable this pass by default and enable the tests back? We can turn it back on when we are confident that all tests pass, and there are no new regressions. Any objections?<br class="">
<br class="">
Thanks,<br class="">
Michael<br class="">
<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>
</div></blockquote></div><br class=""></div></div></blockquote></div></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=""></div></div></body></html>