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

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
<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/fb1dc279/attachment.html>

On Fri, 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?
>
>
How about -fexperimental-global-isel as a flag to clang?

-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> 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/
>
> 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/76b429ee/attachment.html>

> On Jun 16, 2017, at 4:58 PM, Eric Christopher <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!
> 
> -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/ea4bacbf/attachment.html>

On 17 June 2017 at 01:43, Quentin Colombet <qcolombet at apple.com>
wrote:
> *** 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!
While we're at it, these patches could use some review:
* [GlobalISel] combine not symmetric merge/unmerge nodes [1]
* [GlobalISel] Make multi-step legalization work [2]
* [GlobalISel] Enable specifying how to legalize non-power-of-2 size
types. [NFC-ish] [3]

Note that [2] is a spin-off from [3], which has been in review for a
long, long time.

[1] https://reviews.llvm.org/D33626
[2] https://reviews.llvm.org/D32529
[3] https://reviews.llvm.org/D30529

Cheers,
Diana
>
> 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/
>
> 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.

On 19 Jun 2017, at 10:43, Diana Picus via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

On 17 June 2017 at 01:43, Quentin Colombet <qcolombet at
apple.com<mailto:qcolombet at apple.com>> wrote:
*** 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!

While we're at it, these patches could use some review:
* [GlobalISel] combine not symmetric merge/unmerge nodes [1]
* [GlobalISel] Make multi-step legalization work [2]
(https://reviews.llvm.org/D32529)

As the author of the above patch, I think this one should be relatively easy to
review.

* [GlobalISel] Enable specifying how to legalize non-power-of-2 size
types. [NFC-ish] [3] (https://reviews.llvm.org/D30529)

I'm working on rebasing this one on top of the above patch ("Make
multi-step legalization work").
Most of that is done, but I'm still trying to write a better description of
what this large patch actually
does, so that it becomes easier to review. Unfortunately, I haven't found a
way to split that
patch up further in smaller incremental pieces.
I had put this on the back-burner and instead prioritized what was needed to
enable GlobalISel
by default. Now that that is not being pushed as hard, I'll start looking
again at it.
I hope to have an updated patch with a description that makes review easier
towards the end of the week.

Thanks,

Kristof
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170619/17cc927c/attachment.html>

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>

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

Hi Quentin,

My only remaining concern is around ABI compatibility.
The following commit seems to indicate that in the previous round of evaluation,
we didn’t find an existing ABI compatibility issue:
http://llvm.org/viewvc/llvm-project?view=revision&revision=311388.
I haven’t looked into the details of this issue - so maybe I’m worried over
nothing?

I’m wondering if since then on your side you did any testing around ABI
compatibility?
E.g. building software where you semi-randomly build some functions through
GlobalISel and some functions through DAGISel?

Thanks,

Kristof

On 8 Nov 2017, at 00:42, Quentin Colombet via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> 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<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?


*** 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/

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.

_______________________________________________
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/20171113/fac9a9c2/attachment.html>