[llvm] 8e3f59b - [AArch64] Add option to enable/disable load-store renaming.

[Maxim Kuvyrkov via llvm-commits]

On Mon, 17 Feb 2020 at 19:52, Florian Hahn <florian_hahn at apple.com> wrote:

> Hi,
>
> > On 17 Feb 2020, at 17:39, Maxim Kuvyrkov <maxim.kuvyrkov at linaro.org>
> wrote:
> >
> > Hi Florian,
> >
> > This change regresses performance of 464.h264ref (from SPEC CPU2006) by
> 6% on aarch64-linux-gnu.  This happens due to 40% regression in
> FastFullPelBlockMotionSearch().
> >
> > This patch looks like a no-op, but, apparently, it's not.  Is it
> expected that it's not a no-op?
> >
>
> 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.
>
> 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.
>
> For the 10.0 release, I think the option should be disabled to avoid
> shipping with a known issue.
>
> For master, we could turn it on again, which might turn up a better
> reproducer. What do you think?
>
> Cheers,
> Florian
>
>
I agree.  Let's leave conservative fix on release branch and re-enable on
master to increase the probability of catching the bug.

-- 
Maxim Kuvyrkov
www.linaro.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200218/66d08333/attachment.html>