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

Fri Jun 16 17:26:31 PDT 2017

> On Jun 16, 2017, at 5:17 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
> 
> 
> On Fri, Jun 16, 2017 at 5:11 PM Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>> On Jun 16, 2017, at 4:58 PM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>> 
>> 
>> 
>> On Fri, Jun 16, 2017 at 4:43 PM Quentin Colombet <qcolombet at apple.com <mailto: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?
>> 
>> 
>> How about -fexperimental-global-isel as a flag to clang?
> 
> I thought about that and that would work, but I thought we had an implicit contract that clang options are not going away.
> If that’s not the case, then, yes, we should do that!
> 
> *shrug* we have one for the new pass manager to make testing easier, this seems reasonable as well for something that's going to be tested over an extended period of time.

Sounds good to me. We’ll do that.

Thanks!

> 
> -eric
>  
> 
>> 
>> -eric
>>  
>> 
>> *** 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/20170616/7349e3a2/attachment.html>