[llvm-dev] [GlobalISel][AArch64] Toward flipping the switch for O0: Please give it a try!

Thu May 18 00:06:39 PDT 2017

FWIW, I ran Quentin's r299283 Localizer patch (see also http://bugs.llvm.org/show_bug.cgi?id=32561) on my benchmark set.
This is taking the commit in r299283 + adding the pass to the pipeline right after RegBankSelect.

It reduces the slow-down when enabling globalisel at -O0 from 9.5% (on r302453)  to 6.3% (on r302679) in my experiments.
The code size increase also reduces from just over 2.8% to 1.8%.
I haven't measured the impact on compile-time.

I think those are nice improvements; but I wouldn't hold up enabling-by-default for those improvements.
However, the increase in stack size usage being so huge that a bootstrap fails seems like something that should be addressed before enabling-by-default.
I think Diana found that when enabling r299283, the bootstrap failed with llvm-tblgen segfaulting.
So there clearly is some work required there.

Thanks,

Kristof

On 16 May 2017, at 14:06, Diana Picus via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

Turns out it really is a GlobalISel issue - we eat up too much stack
space because all the constants used in the switches are stored on the
stack. We need to fix this somehow before changing the default. I'll
try to give it a run with Quentin's r299283 on top to see if it helps.

Cheers,
Diana

On 15 May 2017 at 09:38, Diana Picus <diana.picus at linaro.org<mailto:diana.picus at linaro.org>> wrote:
Got another one: https://bugs.llvm.org/show_bug.cgi?id=33036

I'm still investigating whether this is an actual GlobalISel issue or
something else (I'll also start a build on a more recent revision to
see how that behaves).

On 12 May 2017 at 20:06, Eric Christopher <echristo at gmail.com<mailto:echristo at gmail.com>> wrote:
Agreed. That was probably just luck before :)

-eric

On Fri, May 12, 2017 at 5:22 AM Diana Picus via llvm-dev
<llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

It turns out that can be fixed by adding -lm to the link line, so I
will probably convert it into a test-suite bug.

I don't suppose it's crucial to handle the fabs intrinsic nicely at -O0.

On 12 May 2017 at 13:44, Diana Picus <diana.picus at linaro.org<mailto:diana.picus at linaro.org>> wrote:
Hi,

I ran into a little snag on the test-suite:
https://bugs.llvm.org/show_bug.cgi?id=33021
It boils down to GlobalISel generating calls to fabs instead of using
FABSDr (so we get undefined references).

Cheers,
Diana

On 11 May 2017 at 18:40, Quentin Colombet <qcolombet at apple.com> wrote:
Hi Diana,

Thanks for the summary.

On May 11, 2017, at 4:01 AM, Diana Picus <diana.picus at linaro.org>
wrote:

Hi all,

I'm still running some validation on this, I'll send an email when
it's done. If that goes well I don't have anything against making the
switch.

For the record, here's a summary of issues that were deferred for
later on (some of which are optimization-ish and we might decide to
never do at -O0 at all):
* Crash in RegBankSelect for half fp types:
https://bugs.llvm.org/show_bug.cgi?id=32560

I’ll have a look.

* Improving constant placement:
http://bugs.llvm.org/show_bug.cgi?id=32561

I’ve commented in the PR to mention the localizer technic I was playing
with, if someone wants to give it a try.

* Fancy switch lowering
* Transforming division-by-constant-power-of-2 into right shift

AFAICT all the other issues that were brought up were fixed (yay!).

Cheers,
Diana

Cheers,
-Quentin

On 11 May 2017 at 08:44, Kristof Beyls via llvm-dev
<llvm-dev at lists.llvm.org> wrote:

On 10 May 2017, at 17:36, Quentin Colombet <qcolombet at apple.com> wrote:

MultiSource/Benchmarks/mediabench/mpeg2/mpeg2dec/mpeg2decode (46%):
Function
Reference_IDCT: Probably due to creating all constants in the entry BB
+
spilling floating point data through an X register:

FastISel:
fadd d0, d1, d0
str d0, [sp,#528]
GlobalISel:
fadd d0, d1, d0
fmov x9, d0
stur x9, [x29,#-48]

Good finding, I forgot to do stores in my previous fix. I’ll do them
shortly.

Should be fixed by r302679

Thanks Quentin,

That reduces the slow-down when enabling globalisel at -O0 from 13% (on
r302453)  to 9.5% (on r302679) in my experiments.
The code size increase also reduces from just over 3% to 2.8%.

Kristof

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170518/94903f85/attachment.html>