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

On 18 Dec 2017, at 15:11, Amara Emerson <aemerson at
apple.com<mailto:aemerson at apple.com>> wrote:


On Dec 18, 2017, at 12:37 PM, Kristof Beyls <Kristof.Beyls at
arm.com<mailto:Kristof.Beyls at arm.com>> wrote:

Hi Amara,

My reasons for preferring the switch to happen after the release branch is based
on the following observations:

  *   As far as I can see, the projects and products following top-of-trunk tend
to test much more extensively than the testing that happens for
llvm.org<http://llvm.org/> releases. I expect that the
llvm.org<http://llvm.org/> release testing won’t find potential remaining
issues, whereas the projects and products following trunk are far more likely to
find remaining issues with GlobalISel. Therefore, I think it’s best to give as
much time as possible for these projects/products following top-of-trunk to find
issues before there’s an llvm.org<http://llvm.org/> release with
GlobalISel enabled-by-default.
  *   For the projects and products that follow top-of-trunk that I know off, if
globalisel-by-default would really break things, it’s possible to disable it
within that project/product without end users of it needing to know about this.
If for the llvm.org<http://llvm.org/> release it would turn out that
globalisel has a big issue after release, we’d need to somehow let all users of
the release know to disable it by adding a convoluted command line option
(‘-mllvm -global-isel=0’?)

Combining the 2 observations above, I think it’s better to do the switch shortly
after a release branch is taken, rather than just before it.

I wasn’t suggesting that we proceed with the full release with GISel enabled by
default if there are serious issues, I was more saying that we have additional
time during the RC period to test further on trunk, and then merge in fixes into
the RC. Worst case is that we back out the change completely but I doubt it’ll
get to that stage. I do agree that trunk sees more of the testing coverage. If
one month isn’t enough, what would have been the cut off point?

Thanks,
Amara

Yeah, I agree that the time needed before the next release happens when
switching is debatable.
After looking it up, I see that the final release tag is planned for no earlier
than 21 February 2018, so we’d indeed have a bit of time to revert it on the
release branch if serious issues were discovered.
Seeing the actual release date is that far out, I don’t have strong objections
to flipping the switch now if others want to press on with it.

Thanks,

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

Ok. We’ll look at what we can do to further stress test it in the next two
months, additional suggestions from the community is welcome. Patch should be
incoming to enable it later today.

Thanks,
Amara
> On Dec 18, 2017, at 3:25 PM, Kristof Beyls <Kristof.Beyls at arm.com>
wrote:
> 
>> 
>> On 18 Dec 2017, at 15:11, Amara Emerson <aemerson at apple.com
<mailto:aemerson at apple.com>> wrote:
>> 
>> 
>>> On Dec 18, 2017, at 12:37 PM, Kristof Beyls <Kristof.Beyls at
arm.com <mailto:Kristof.Beyls at arm.com>> wrote:
>>> 
>>> Hi Amara,
>>> 
>>> My reasons for preferring the switch to happen after the release
branch is based on the following observations:
>>> As far as I can see, the projects and products following
top-of-trunk tend to test much more extensively than the testing that happens
for llvm.org <http://llvm.org/> releases. I expect that the llvm.org
<http://llvm.org/> release testing won’t find potential remaining issues,
whereas the projects and products following trunk are far more likely to find
remaining issues with GlobalISel. Therefore, I think it’s best to give as much
time as possible for these projects/products following top-of-trunk to find
issues before there’s an llvm.org <http://llvm.org/> release with
GlobalISel enabled-by-default.
>>> For the projects and products that follow top-of-trunk that I know
off, if globalisel-by-default would really break things, it’s possible to
disable it within that project/product without end users of it needing to know
about this. If for the llvm.org <http://llvm.org/> release it would turn
out that globalisel has a big issue after release, we’d need to somehow let all
users of the release know to disable it by adding a convoluted command line
option (‘-mllvm -global-isel=0’?)
>>> Combining the 2 observations above, I think it’s better to do the
switch shortly after a release branch is taken, rather than just before it.
>>> 
>> I wasn’t suggesting that we proceed with the full release with GISel
enabled by default if there are serious issues, I was more saying that we have
additional time during the RC period to test further on trunk, and then merge in
fixes into the RC. Worst case is that we back out the change completely but I
doubt it’ll get to that stage. I do agree that trunk sees more of the testing
coverage. If one month isn’t enough, what would have been the cut off point?
>> 
>> Thanks,
>> Amara
> 
> Yeah, I agree that the time needed before the next release happens when
switching is debatable.
> After looking it up, I see that the final release tag is planned for no
earlier than 21 February 2018, so we’d indeed have a bit of time to revert it on
the release branch if serious issues were discovered.
> Seeing the actual release date is that far out, I don’t have strong
objections to flipping the switch now if others want to press on with it.
> 
> Thanks,
> 
> Kristof
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171218/8749f0d5/attachment.html>

FYI all: the patch to enable it is available to review here:
https://reviews.llvm.org/D41362

Thanks,
Amara
> On Dec 18, 2017, at 5:44 PM, Amara Emerson via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Ok. We’ll look at what we can do to further stress test it in the next two
months, additional suggestions from the community is welcome. Patch should be
incoming to enable it later today.
> 
> Thanks,
> Amara
> 
>> On Dec 18, 2017, at 3:25 PM, Kristof Beyls <Kristof.Beyls at arm.com
<mailto:Kristof.Beyls at arm.com>> wrote:
>> 
>>> 
>>> On 18 Dec 2017, at 15:11, Amara Emerson <aemerson at apple.com
<mailto:aemerson at apple.com>> wrote:
>>> 
>>> 
>>>> On Dec 18, 2017, at 12:37 PM, Kristof Beyls <Kristof.Beyls
at arm.com <mailto:Kristof.Beyls at arm.com>> wrote:
>>>> 
>>>> Hi Amara,
>>>> 
>>>> My reasons for preferring the switch to happen after the
release branch is based on the following observations:
>>>> As far as I can see, the projects and products following
top-of-trunk tend to test much more extensively than the testing that happens
for llvm.org <http://llvm.org/> releases. I expect that the llvm.org
<http://llvm.org/> release testing won’t find potential remaining issues,
whereas the projects and products following trunk are far more likely to find
remaining issues with GlobalISel. Therefore, I think it’s best to give as much
time as possible for these projects/products following top-of-trunk to find
issues before there’s an llvm.org <http://llvm.org/> release with
GlobalISel enabled-by-default.
>>>> For the projects and products that follow top-of-trunk that I
know off, if globalisel-by-default would really break things, it’s possible to
disable it within that project/product without end users of it needing to know
about this. If for the llvm.org <http://llvm.org/> release it would turn
out that globalisel has a big issue after release, we’d need to somehow let all
users of the release know to disable it by adding a convoluted command line
option (‘-mllvm -global-isel=0’?)
>>>> Combining the 2 observations above, I think it’s better to do
the switch shortly after a release branch is taken, rather than just before it.
>>>> 
>>> I wasn’t suggesting that we proceed with the full release with
GISel enabled by default if there are serious issues, I was more saying that we
have additional time during the RC period to test further on trunk, and then
merge in fixes into the RC. Worst case is that we back out the change completely
but I doubt it’ll get to that stage. I do agree that trunk sees more of the
testing coverage. If one month isn’t enough, what would have been the cut off
point?
>>> 
>>> Thanks,
>>> Amara
>> 
>> Yeah, I agree that the time needed before the next release happens when
switching is debatable.
>> After looking it up, I see that the final release tag is planned for no
earlier than 21 February 2018, so we’d indeed have a bit of time to revert it on
the release branch if serious issues were discovered.
>> Seeing the actual release date is that far out, I don’t have strong
objections to flipping the switch now if others want to press on with it.
>> 
>> Thanks,
>> 
>> Kristof
> 
> _______________________________________________
> LLVM Developers mailing list
> 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/20171218/e31a90dc/attachment.html>