<div dir="ltr"><div dir="ltr">On Mon, 17 Feb 2020 at 19:52, Florian Hahn <<a href="mailto:florian_hahn@apple.com" target="_blank">florian_hahn@apple.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
> On 17 Feb 2020, at 17:39, Maxim Kuvyrkov <<a href="mailto:maxim.kuvyrkov@linaro.org" target="_blank">maxim.kuvyrkov@linaro.org</a>> wrote:<br>
> <br>
> Hi Florian,<br>
> <br>
> This change regresses performance of 464.h264ref (from SPEC CPU2006) by 6% on aarch64-linux-gnu.  This happens due to 40% regression in FastFullPelBlockMotionSearch().<br>
> <br>
> This patch looks like a no-op, but, apparently, it's not.  Is it expected that it's not a no-op?<br>
> <br>
<br>
Yes I think that is expected. This patch adds an option to disable the register renaming in the AArch64 load/store optimiser (which got added around November/December 2019) and disables it by default. This means fewer store pairs are generated probably causing the regression. Enabling aarch64-load-store-renaming should restore the performance.<br>
<br>
The reason for disabling it is that we found a mis-compile caused by the renaming. I am still looking into tracking down the mis-compile, but unfortunately the mis-compile is in an internal app that is quite tricky to build.<br>
<br>
For the 10.0 release, I think the option should be disabled to avoid shipping with a known issue.<br>
<br>
For master, we could turn it on again, which might turn up a better reproducer. What do you think?<br>
<br>
Cheers,<br>
Florian <br>
<br>
</blockquote></div><div><br></div><div>I agree.  Let's leave conservative fix on release branch and re-enable on master to increase the probability of catching the bug.</div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Maxim Kuvyrkov</div><div><a href="http://www.linaro.org" target="_blank">www.linaro.org</a></div></div></div></div>