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

Tue Nov 7 16:42:11 PST 2017

Hi all,

I’d like to resurrect this thread and ask if people are on board for enabling this by default for AArch64 O0.

*** What Changed Since June? ***

- We added a way to describe the legalization actions for non-power-of-2 
- We gave a tutorial that covers the best practices to target GlobalISel
- We improved the TableGen backend to reuse existing SDISel patterns
- We built and ran huge internal software with GISel
- We evaluated the performance of GISel and are confident things are in a good shape (with https://reviews.llvm.org/D39034) and moving forward would look even better (see the last LLVM Dev talk: GlobalISel: Present, Past, and Future when it is available)

*** So What’s he Plan? ***

- Switch the default instruction selector to GISel for AArch64 at O0
- Enable the fallback path by default for AArch64 (with warnings enabled when that path is hit)
- Provide a clang option to turn GISel off

What do you think?

Thanks,
-Quentin

> On Jun 16, 2017, at 4:43 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> Hi all,
> 
> We had some internal discussions about flipping the default for O0 and we concluded that we wanted to postpone it.
> 
> 
> *** Why Is That? ***
> 
> We don’t want to send the wrong message that GlobalISel’s design is set in stone and ready for broader adoption.
> In particular,
> 1. The APIs are still evolving and can still possibly change significantly
> 2. The TableGen backend to reuse the existing SD patterns is still at its early stage
> 3. We want to investigate closely the performance of global-isel (compile-time, runtime, code size, fallbacks)
> 
> The rationale behind those items is that we want to minimize the pain of moving forward for everybody. We also want the out-of-the-box experience to be pleasant (like all/most of the tablegen patterns just work, we have documentation on how to target a new backend, etc.) Finally, we want to gain confidence we are going to be able to address the performance issues we have with the current design and if not, derive a plan for that.
> 
> We purposely left out of the conversation what will be the right time and requirements to flip the switch. We want to gather more data first. Your help would be appreciated!
> 
> 
> *** Short-Term Proposal ***
> 
> What we would like to do instead short-term is:
> A. Repurpose or create an option “-aarch64-enable-global-isel-at-O” to enable GISel with fallbacks and warnings enables (i.e., equivalent of -global-isel -global-isel-abort=2)
> B. Advertise this option in the next open source release to allow compiler enthusiastic to try it and report problems
> C. Have GISel always built so we can push thing in the right place, MachineVerifier in mind, and stop doing some weird gymnastic
> 
> What do people think?
> 
> 
> *** Your Help Is Needed ***
> 
> - Please share your experience in using the GISel APIs and how we can make them better. Moving forward we’ll have those conversations on open source instead of internally/with a narrower audience.
> - Report any performance problem you identify
> - Propose patches!
> 
> Cheers,
> -Quentin
> 
> 
> 
>> On Jun 16, 2017, at 3:06 PM, Quentin Colombet via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>>> 
>>> On Jun 14, 2017, at 7:27 AM, Diana Picus <diana.picus at linaro.org <mailto:diana.picus at linaro.org>> wrote:
>>> 
>>> On 12 June 2017 at 18:54, Diana Picus <diana.picus at linaro.org <mailto:diana.picus at linaro.org>> wrote:
>>> Hi all,
>>> 
>>> I added a buildbot [1] running the test-suite with -O0 -global-isel. It runs into the same 2 timeouts that I reported previously on this thread (paq8p and scimark2). It would be nice to make it green before flipping the switch.
>>> 
>>> 
>>> I did some more investigations on a machine similar to the one running the buildbot. For paq8p and scimark2, I get these results for O0:
>>> 
>>> PAQ8p:
>>> Fast isel: 666.344
>>> Global isel: 731.384
>>> 
>>> SciMark2-C:
>>> Fast isel: 463.908
>>> Global isel: 496.22
>>> 
>>> The current timeout is 500s (so in this particular case we didn't hit it for scimark2, and it ran successfully to completion). I don't think the difference between FastISel and GlobalISel is too atrocious, so I would propose increasing the timeout for these 2 benchmarks. I'm not sure if we can do this on a per-bot basis, but I see some precedent for setting custom timeout thresholds for various benchmarks on different architectures (sometimes with comments that it's done so we can run O0 on that particular benchmark). 
>>> 
>>> Something along these lines works:
>>> https://reviews.llvm.org/differential/diff/102547/ <https://reviews.llvm.org/differential/diff/102547/>
>>> 
>>> What do you guys think about this approach?
>> 
>> Looks reasonable to me.
>> 
>>> 
>>> Thanks,
>>> Diana
>>> 
>>> PS: The buildbot is using the Makefiles because that's what our other AArch64 test-suite bots use. Moving all of them to CMake is a transition for another time.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171107/71c64ce2/attachment.html>