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

[via llvm-dev]

SGTM, Quentin.

On 2017-11-07 19:42, Quentin Colombet via llvm-dev wrote:
> 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> wrote:
>> 
>> On Jun 14, 2017, at 7:27 AM, Diana Picus <diana.picus at linaro.org>
>> wrote:
>> 
>> On 12 June 2017 at 18:54, Diana Picus <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/ [1]
>> 
>> 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.
> 
> 
> 
> Links:
> ------
> [1] https://reviews.llvm.org/differential/diff/102547/
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev