Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Szabolcs Nagy]

The 04/30/2020 12:26, Kyrylo Tkachov wrote:
> > > From: Gcc  On Behalf Of Andrew Pinski via Gcc
> > > On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
> > > wrote:
> > > > Distributions are receiving requests to build things with
> > > > -moutline-atomics:
> > > >
> > > >   
> > > >
> > > > Should this be reflected in the GCC upstream defaults for ARMv8-A
> > > > generic tuning?  It does not make much sense to me if every distribution
> > > > has to overide these flags, either in their build system or by patching
> > > > GCC.
> > >
> > > At least we should make it a configure option.
> > > I do want the ability to default it for our (Marvell) toolchain for
> > > Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> > > anyways).
> > 
> > After some internal discussions, I am open to having it on as a default.
> > Here are two versions. One has it as a tuning setting that CPUs can 
> > override,
> > the other just switches it on by default always unless overridden by -mno-
> > outline-atomics.
> > I slightly prefer the second one as it's cleaner and simpler, but happy to 
> > take
> > either.
> > Any preferences?
> > Thanks,
> > Kyrill
> > 
> > ChangeLogs:
> > 
> > 2020-04-30  Kyrylo Tkachov  
> > 
> > * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics):
> > Declare.
> > * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
> > * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> > variable.
> > 
> > 2020-04-30  Kyrylo Tkachov  
> > 
> > * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
> > * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> > variable.
> > * doc/invoke.texi (moutline-atomics): Document as on by default.
> 
> I've pushed this second variant after bootstrap and testing on 
> aarch64-none-linux-gnu.
> Compiled a simple atomic-using testcase with all relevant combinations of 
> -moutline-atomics and LSE and no-LSE -march options.
> Before the results were (as expected):
>  |-moutline-atomics | -mno-outline-atomics |  option
> 
> LSE  |  inline LSE  | inline LSE| inline LSE
> no-LSE   |  outline | inline LDXR/STXR  | inline LDX/STXR
> 
> 
> with this patch they are:
>  -moutline-atomics  | -mno-outline-atomics |  option>
> 
> LSE  |  inline LSE  | inline LSE   | inline LSE
> no-LSE   |  outline | inline LDXR/STXR | outline

note that this change affects all aarch64 targets not just *-gnu,
(baremetal, freebsd, musl,..), however on non-gnu targets the
initializer with __getauxval is missing so the outlined calls
always use baseline LDXR/STXR.

in a given elf module the libgcc initializer may not run first
(e.g. it runs after ifunc resolvers, other initializers or
signal handler) and different objects may be compiled with
different outline-atomics setting so the outlined calls must
be abi compatible with LDXR/STXR atomics (i.e. outlining is not
an atomics abi change).

so presumably non-gnu targets want to build gcc with outline
atomics turned off by default or want a mechanism to propagate
the lse hwcap to libgcc that works other than in glibc.

I opened
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95128
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95129

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Andrew Haley via Gcc-patches]

On 5/1/20 11:48 AM, JiangNing OS via Gcc-patches wrote:
> In reality, a lot of users are still using old gcc versions running on new 
> hardware. OpenJDK is a typical example, I think.

We can change the OpenJDK build scripts to use -moutline-atomics if
it's available. I agree with Richard that we should not change codegen
for an existing GCC release series unless there is a bug.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Kyrylo Tkachov]

Hi all,

> -Original Message-
> From: Florian Weimer 
> Sent: 04 May 2020 09:35
> To: Joseph Myers 
> Cc: Kyrylo Tkachov ; Andrew Pinski
> ; gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> * Joseph Myers:
> 
> > I think this change is what introduced a glibc testsuite regression in the
> > localplt test.
> >
> > https://sourceware.org/pipermail/libc-testresults/2020q2/006097.html
> >
> > Contents of elf/check-localplt.out:
> >
> > Extra PLT reference: libc.so: getauxval
> >
> > A reference to getauxval from libgcc is also a namespace violation, since
> > that name is in the user's namespace and to users may define it with their
> > own semantics.  glibc deliberately provides __getauxval at a public symbol
> > version for use in libgcc.  (But once libgcc is using __getauxval, glibc
> > will probably still need updating to allow a local PLT reference to
> > __getauxval from libc.so, as libgcc code that gets statically linked into
> > libc.so can't use glibc's internal hidden symbol conventions because it
> > also gets used outside of libc.so.)
> 
> I think you are are right.  From libgcc/config/aarch64/lse-init.c:
> 
> static void __attribute__((constructor))
> init_have_lse_atomics (void)
> {
>   unsigned long hwcap = getauxval (AT_HWCAP);
>   __aarch64_have_lse_atomics = (hwcap & HWCAP_ATOMICS) != 0;
> }
> 
> I should have seen this as the original patches went in, sorry.
> 
> The question, of course, is if we expect people to call __getauxval, why
> do we not declare it in the header file?  And with sufficient compiler
> support, redirect callers to that alias?
> 
> The same question applies to __secure_getenv, where we actually moved in
> the other direction!
> 
> This never had made sense to me, and it still does not.

Thanks Joseph for reporting this, I'm testing a patch to replace getauxval with 
__getauxval.
So far it passed GCC bootstrap and testing (I haven't tried the glibc tests 
yet).
Is that the desirable fix at this point in GCC?
Thanks,
Kyrill

> 
> Thanks,
> Florian

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Florian Weimer via Gcc-patches]

* Joseph Myers:

> I think this change is what introduced a glibc testsuite regression in the 
> localplt test.
>
> https://sourceware.org/pipermail/libc-testresults/2020q2/006097.html
>
> Contents of elf/check-localplt.out:
>
> Extra PLT reference: libc.so: getauxval
>
> A reference to getauxval from libgcc is also a namespace violation, since 
> that name is in the user's namespace and to users may define it with their 
> own semantics.  glibc deliberately provides __getauxval at a public symbol 
> version for use in libgcc.  (But once libgcc is using __getauxval, glibc 
> will probably still need updating to allow a local PLT reference to 
> __getauxval from libc.so, as libgcc code that gets statically linked into 
> libc.so can't use glibc's internal hidden symbol conventions because it 
> also gets used outside of libc.so.)

I think you are are right.  From libgcc/config/aarch64/lse-init.c:

static void __attribute__((constructor))
init_have_lse_atomics (void)
{
  unsigned long hwcap = getauxval (AT_HWCAP);
  __aarch64_have_lse_atomics = (hwcap & HWCAP_ATOMICS) != 0;
}

I should have seen this as the original patches went in, sorry.

The question, of course, is if we expect people to call __getauxval, why
do we not declare it in the header file?  And with sufficient compiler
support, redirect callers to that alias?

The same question applies to __secure_getenv, where we actually moved in
the other direction!

This never had made sense to me, and it still does not.

Thanks,
Florian

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Joseph Myers]

I think this change is what introduced a glibc testsuite regression in the 
localplt test.

https://sourceware.org/pipermail/libc-testresults/2020q2/006097.html

Contents of elf/check-localplt.out:

Extra PLT reference: libc.so: getauxval

A reference to getauxval from libgcc is also a namespace violation, since 
that name is in the user's namespace and to users may define it with their 
own semantics.  glibc deliberately provides __getauxval at a public symbol 
version for use in libgcc.  (But once libgcc is using __getauxval, glibc 
will probably still need updating to allow a local PLT reference to 
__getauxval from libc.so, as libgcc code that gets statically linked into 
libc.so can't use glibc's internal hidden symbol conventions because it 
also gets used outside of libc.so.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Jeff Law via Gcc-patches]

On Fri, 2020-05-01 at 12:52 +0100, Richard Earnshaw wrote:
> On 01/05/2020 12:01, Kyrylo Tkachov wrote:
> > Hi JangNing (please reply inline in the future as that is the preferred 
> > style
> > on this mailing list)
> > 
> > > -Original Message-
> > > From: JiangNing OS 
> > > Sent: 01 May 2020 11:49
> > > To: Richard Earnshaw ; Kyrylo Tkachov
> > > ; Andrew Pinski ; Florian
> > > Weimer 
> > > Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> > > Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> > > 
> > > In reality, a lot of users are still using old gcc versions running on new
> > > hardware. OpenJDK is a typical example, I think.
> > 
> > Although this option is not an ABI change and thus I don't expect it to 
> > break
> > anything by design, it is definitely good to be cautious as it is a
> > significant change in code generation. Let's wait for a few weeks to make
> > sure the default change in GCC 10.1 works for everyone and then revisit the
> > backport story. In any case, GCC 9 and 8 are not extremely close to release
> > currently IIUC.
> 
> Frankly, I don't think there is anything to consider.  We should not
> change codegen for an existing release series unless there is a bug.  It
> upsets users who expect stability in dot releases.
> 
> There's an option that can be used to change the behaviour, and that's
> enough.
I tend to agree.  ISTM that the best course of action for the older releases and
distros is to have the options available and the distros can determine on their
own if they want to flip the default by configuring GCC differently or just 
build
packages with the magic option.

Jeff

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Richard Earnshaw]

On 01/05/2020 12:01, Kyrylo Tkachov wrote:
> Hi JangNing (please reply inline in the future as that is the preferred style 
> on this mailing list)
> 
>> -Original Message-
>> From: JiangNing OS 
>> Sent: 01 May 2020 11:49
>> To: Richard Earnshaw ; Kyrylo Tkachov
>> ; Andrew Pinski ; Florian
>> Weimer 
>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
>> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
>>
>> In reality, a lot of users are still using old gcc versions running on new
>> hardware. OpenJDK is a typical example, I think.
> 
> Although this option is not an ABI change and thus I don't expect it to break 
> anything by design, it is definitely good to be cautious as it is a 
> significant change in code generation. Let's wait for a few weeks to make 
> sure the default change in GCC 10.1 works for everyone and then revisit the 
> backport story. In any case, GCC 9 and 8 are not extremely close to release 
> currently IIUC.

Frankly, I don't think there is anything to consider.  We should not
change codegen for an existing release series unless there is a bug.  It
upsets users who expect stability in dot releases.

There's an option that can be used to change the behaviour, and that's
enough.

R.

> 
> Thanks,
> Kyrill
> 
>>
>>> -Original Message-
>>> From: Richard Earnshaw 
>>> Sent: Friday, May 1, 2020 6:41 PM
>>> To: JiangNing OS ; Kyrylo Tkachov
>>> ; Andrew Pinski ; Florian
>>> Weimer 
>>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
>>> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
>>>
>>> On 01/05/2020 11:38, JiangNing OS via Gcc-patches wrote:
>>>> Hi Kyrill,
>>>>
>>>> Can it be backported to gcc 8/9/10 branches?
>>>>
>>>
>>> I'm not sure changing the defaults of things like this is a good idea on 
>>> 'dot'
>>> releases.
>>>
>>> Having the option is one thing.  Changing the default another thing 
>>> entirely.
>>>
>>> R.
>>>
>>>> Thanks,
>>>> -Jiangning
>>>>
>>>>> -Original Message-
>>>>> From: Gcc-patches  On Behalf Of
>>>>> Kyrylo Tkachov
>>>>> Sent: Thursday, April 30, 2020 8:27 PM
>>>>> To: Kyrylo Tkachov ; Andrew Pinski
>>>>> ; Florian Weimer 
>>>>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
>>>>> Subject: RE: Should ARMv8-A generic tuning default to
>>>>> -moutline-atomics
>>>>>
>>>>>
>>>>>
>>>>>> -Original Message-----
>>>>>> From: Gcc-patches  On Behalf Of
>>>>>> Kyrylo Tkachov
>>>>>> Sent: 30 April 2020 11:57
>>>>>> To: Andrew Pinski ; Florian Weimer
>>>>>> 
>>>>>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
>>>>>> Subject: RE: Should ARMv8-A generic tuning default to
>>>>>> -moutline-atomics
>>>>>>
>>>>>> [Moving to gcc-patches]
>>>>>>
>>>>>>> -Original Message-
>>>>>>> From: Gcc  On Behalf Of Andrew Pinski
>> via
>>>>>>> Gcc
>>>>>>> Sent: 30 April 2020 07:21
>>>>>>> To: Florian Weimer 
>>>>>>> Cc: GCC Mailing List ; nmeye...@amzn.com
>>>>>>> Subject: Re: Should ARMv8-A generic tuning default to
>>>>>>> -moutline-atomics
>>>>>>>
>>>>>>> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc
>>>>>>> 
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Distributions are receiving requests to build things with
>>>>>>>> -moutline-atomics:
>>>>>>>>
>>>>>>>>   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
>>>>>>>>
>>>>>>>> Should this be reflected in the GCC upstream defaults for ARMv8-A
>>>>>>>> generic tuning?  It does not make much sense to me if every
>>>>>>>> distribution has to overide these flags, either in their build
>>>>>>>> system or by patching GCC.
>>>>>>>
>>>>>>> At least we should make it a configure option.
>>>>>>> I do want the ability to default it for our (M

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Kyrylo Tkachov]

Hi JangNing (please reply inline in the future as that is the preferred style 
on this mailing list)

> -Original Message-
> From: JiangNing OS 
> Sent: 01 May 2020 11:49
> To: Richard Earnshaw ; Kyrylo Tkachov
> ; Andrew Pinski ; Florian
> Weimer 
> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> In reality, a lot of users are still using old gcc versions running on new
> hardware. OpenJDK is a typical example, I think.

Although this option is not an ABI change and thus I don't expect it to break 
anything by design, it is definitely good to be cautious as it is a significant 
change in code generation. Let's wait for a few weeks to make sure the default 
change in GCC 10.1 works for everyone and then revisit the backport story. In 
any case, GCC 9 and 8 are not extremely close to release currently IIUC.

Thanks,
Kyrill

> 
> > -Original Message-
> > From: Richard Earnshaw 
> > Sent: Friday, May 1, 2020 6:41 PM
> > To: JiangNing OS ; Kyrylo Tkachov
> > ; Andrew Pinski ; Florian
> > Weimer 
> > Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> > Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> >
> > On 01/05/2020 11:38, JiangNing OS via Gcc-patches wrote:
> > > Hi Kyrill,
> > >
> > > Can it be backported to gcc 8/9/10 branches?
> > >
> >
> > I'm not sure changing the defaults of things like this is a good idea on 
> > 'dot'
> > releases.
> >
> > Having the option is one thing.  Changing the default another thing 
> > entirely.
> >
> > R.
> >
> > > Thanks,
> > > -Jiangning
> > >
> > >> -Original Message-
> > >> From: Gcc-patches  On Behalf Of
> > >> Kyrylo Tkachov
> > >> Sent: Thursday, April 30, 2020 8:27 PM
> > >> To: Kyrylo Tkachov ; Andrew Pinski
> > >> ; Florian Weimer 
> > >> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> > >> Subject: RE: Should ARMv8-A generic tuning default to
> > >> -moutline-atomics
> > >>
> > >>
> > >>
> > >>> -Original Message-
> > >>> From: Gcc-patches  On Behalf Of
> > >>> Kyrylo Tkachov
> > >>> Sent: 30 April 2020 11:57
> > >>> To: Andrew Pinski ; Florian Weimer
> > >>> 
> > >>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> > >>> Subject: RE: Should ARMv8-A generic tuning default to
> > >>> -moutline-atomics
> > >>>
> > >>> [Moving to gcc-patches]
> > >>>
> > >>>> -Original Message-
> > >>>> From: Gcc  On Behalf Of Andrew Pinski
> via
> > >>>> Gcc
> > >>>> Sent: 30 April 2020 07:21
> > >>>> To: Florian Weimer 
> > >>>> Cc: GCC Mailing List ; nmeye...@amzn.com
> > >>>> Subject: Re: Should ARMv8-A generic tuning default to
> > >>>> -moutline-atomics
> > >>>>
> > >>>> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc
> > >>>> 
> > >>>> wrote:
> > >>>>>
> > >>>>> Distributions are receiving requests to build things with
> > >>>>> -moutline-atomics:
> > >>>>>
> > >>>>>   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
> > >>>>>
> > >>>>> Should this be reflected in the GCC upstream defaults for ARMv8-A
> > >>>>> generic tuning?  It does not make much sense to me if every
> > >>>>> distribution has to overide these flags, either in their build
> > >>>>> system or by patching GCC.
> > >>>>
> > >>>> At least we should make it a configure option.
> > >>>> I do want the ability to default it for our (Marvell) toolchain for
> > >>>> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> > >>>> anyways).
> > >>>
> > >>> After some internal discussions, I am open to having it on as a default.
> > >>> Here are two versions. One has it as a tuning setting that CPUs can
> > >>> override, the other just switches it on by default always unless
> > >>> overridden by -mno- outline-atomics.
> > >>> I slightly prefer the second one as it's cleane

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[JiangNing OS via Gcc-patches]

In reality, a lot of users are still using old gcc versions running on new 
hardware. OpenJDK is a typical example, I think.

> -Original Message-
> From: Richard Earnshaw 
> Sent: Friday, May 1, 2020 6:41 PM
> To: JiangNing OS ; Kyrylo Tkachov
> ; Andrew Pinski ; Florian
> Weimer 
> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> On 01/05/2020 11:38, JiangNing OS via Gcc-patches wrote:
> > Hi Kyrill,
> >
> > Can it be backported to gcc 8/9/10 branches?
> >
> 
> I'm not sure changing the defaults of things like this is a good idea on 'dot'
> releases.
> 
> Having the option is one thing.  Changing the default another thing entirely.
> 
> R.
> 
> > Thanks,
> > -Jiangning
> >
> >> -Original Message-
> >> From: Gcc-patches  On Behalf Of
> >> Kyrylo Tkachov
> >> Sent: Thursday, April 30, 2020 8:27 PM
> >> To: Kyrylo Tkachov ; Andrew Pinski
> >> ; Florian Weimer 
> >> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> >> Subject: RE: Should ARMv8-A generic tuning default to
> >> -moutline-atomics
> >>
> >>
> >>
> >>> -Original Message-
> >>> From: Gcc-patches  On Behalf Of
> >>> Kyrylo Tkachov
> >>> Sent: 30 April 2020 11:57
> >>> To: Andrew Pinski ; Florian Weimer
> >>> 
> >>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> >>> Subject: RE: Should ARMv8-A generic tuning default to
> >>> -moutline-atomics
> >>>
> >>> [Moving to gcc-patches]
> >>>
> >>>> -Original Message-
> >>>> From: Gcc  On Behalf Of Andrew Pinski via
> >>>> Gcc
> >>>> Sent: 30 April 2020 07:21
> >>>> To: Florian Weimer 
> >>>> Cc: GCC Mailing List ; nmeye...@amzn.com
> >>>> Subject: Re: Should ARMv8-A generic tuning default to
> >>>> -moutline-atomics
> >>>>
> >>>> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc
> >>>> 
> >>>> wrote:
> >>>>>
> >>>>> Distributions are receiving requests to build things with
> >>>>> -moutline-atomics:
> >>>>>
> >>>>>   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
> >>>>>
> >>>>> Should this be reflected in the GCC upstream defaults for ARMv8-A
> >>>>> generic tuning?  It does not make much sense to me if every
> >>>>> distribution has to overide these flags, either in their build
> >>>>> system or by patching GCC.
> >>>>
> >>>> At least we should make it a configure option.
> >>>> I do want the ability to default it for our (Marvell) toolchain for
> >>>> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> >>>> anyways).
> >>>
> >>> After some internal discussions, I am open to having it on as a default.
> >>> Here are two versions. One has it as a tuning setting that CPUs can
> >>> override, the other just switches it on by default always unless
> >>> overridden by -mno- outline-atomics.
> >>> I slightly prefer the second one as it's cleaner and simpler, but
> >>> happy to take either.
> >>> Any preferences?
> >>> Thanks,
> >>> Kyrill
> >>>
> >>> ChangeLogs:
> >>>
> >>> 2020-04-30  Kyrylo Tkachov  
> >>>
> >>>   * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics):
> >>> Declare.
> >>>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
> >>>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> >>> variable.
> >>>
> >>> 2020-04-30  Kyrylo Tkachov  
> >>>
> >>>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
> >>>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> >>> variable.
> >>>   * doc/invoke.texi (moutline-atomics): Document as on by default.
> >>>
> >>
> >> I've pushed this second variant after bootstrap and testing on
> >> aarch64-none- linux-gnu.
> >> Compiled a simple atomic-using testcase with all relevant
> >> combinations of - moutline-atomics and LSE and no-LSE -march options.
> >> Before the results were (as expected):
> >>  |-moutline-atomics | -mno-outline-atomics |  >> outline-atomics option
> >> 
> >> LSE  |  inline LSE  | inline LSE| inline LSE
> >> no-LSE   |  outline | inline LDXR/STXR  | inline LDX/STXR
> >>
> >>
> >> with this patch they are:
> >>  -moutline-atomics  | -mno-outline-atomics |  >> outline-atomics option>
> >> 
> >> LSE  |  inline LSE  | inline LSE   | inline LSE
> >> no-LSE   |  outline | inline LDXR/STXR | outline
> >>
> >> Thanks,
> >> Kyrill

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Richard Earnshaw]

On 01/05/2020 11:38, JiangNing OS via Gcc-patches wrote:
> Hi Kyrill,
> 
> Can it be backported to gcc 8/9/10 branches?
> 

I'm not sure changing the defaults of things like this is a good idea on
'dot' releases.

Having the option is one thing.  Changing the default another thing
entirely.

R.

> Thanks,
> -Jiangning
> 
>> -Original Message-
>> From: Gcc-patches  On Behalf Of Kyrylo
>> Tkachov
>> Sent: Thursday, April 30, 2020 8:27 PM
>> To: Kyrylo Tkachov ; Andrew Pinski
>> ; Florian Weimer 
>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
>> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
>>
>>
>>
>>> -Original Message-
>>> From: Gcc-patches  On Behalf Of
>>> Kyrylo Tkachov
>>> Sent: 30 April 2020 11:57
>>> To: Andrew Pinski ; Florian Weimer
>>> 
>>> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
>>> Subject: RE: Should ARMv8-A generic tuning default to
>>> -moutline-atomics
>>>
>>> [Moving to gcc-patches]
>>>
>>>> -----Original Message-
>>>> From: Gcc  On Behalf Of Andrew Pinski via
>>>> Gcc
>>>> Sent: 30 April 2020 07:21
>>>> To: Florian Weimer 
>>>> Cc: GCC Mailing List ; nmeye...@amzn.com
>>>> Subject: Re: Should ARMv8-A generic tuning default to
>>>> -moutline-atomics
>>>>
>>>> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc
>>>> 
>>>> wrote:
>>>>>
>>>>> Distributions are receiving requests to build things with
>>>>> -moutline-atomics:
>>>>>
>>>>>   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
>>>>>
>>>>> Should this be reflected in the GCC upstream defaults for ARMv8-A
>>>>> generic tuning?  It does not make much sense to me if every
>>>>> distribution has to overide these flags, either in their build
>>>>> system or by patching GCC.
>>>>
>>>> At least we should make it a configure option.
>>>> I do want the ability to default it for our (Marvell) toolchain for
>>>> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
>>>> anyways).
>>>
>>> After some internal discussions, I am open to having it on as a default.
>>> Here are two versions. One has it as a tuning setting that CPUs can
>>> override, the other just switches it on by default always unless
>>> overridden by -mno- outline-atomics.
>>> I slightly prefer the second one as it's cleaner and simpler, but
>>> happy to take either.
>>> Any preferences?
>>> Thanks,
>>> Kyrill
>>>
>>> ChangeLogs:
>>>
>>> 2020-04-30  Kyrylo Tkachov  
>>>
>>> * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics):
>>> Declare.
>>> * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>>> * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
>>> variable.
>>>
>>> 2020-04-30  Kyrylo Tkachov  
>>>
>>> * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>>> * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
>>> variable.
>>> * doc/invoke.texi (moutline-atomics): Document as on by default.
>>>
>>
>> I've pushed this second variant after bootstrap and testing on aarch64-none-
>> linux-gnu.
>> Compiled a simple atomic-using testcase with all relevant combinations of -
>> moutline-atomics and LSE and no-LSE -march options.
>> Before the results were (as expected):
>>  |-moutline-atomics | -mno-outline-atomics | > option
>> 
>> LSE  |  inline LSE  | inline LSE| inline LSE
>> no-LSE   |  outline | inline LDXR/STXR  | inline LDX/STXR
>>
>>
>> with this patch they are:
>>  -moutline-atomics  | -mno-outline-atomics | > option>
>> 
>> LSE  |  inline LSE  | inline LSE   | inline LSE
>> no-LSE   |  outline | inline LDXR/STXR | outline
>>
>> Thanks,
>> Kyrill

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[JiangNing OS via Gcc-patches]

Hi Kyrill,

Can it be backported to gcc 8/9/10 branches?

Thanks,
-Jiangning

> -Original Message-
> From: Gcc-patches  On Behalf Of Kyrylo
> Tkachov
> Sent: Thursday, April 30, 2020 8:27 PM
> To: Kyrylo Tkachov ; Andrew Pinski
> ; Florian Weimer 
> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> 
> 
> > -Original Message-
> > From: Gcc-patches  On Behalf Of
> > Kyrylo Tkachov
> > Sent: 30 April 2020 11:57
> > To: Andrew Pinski ; Florian Weimer
> > 
> > Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> > Subject: RE: Should ARMv8-A generic tuning default to
> > -moutline-atomics
> >
> > [Moving to gcc-patches]
> >
> > > -Original Message-
> > > From: Gcc  On Behalf Of Andrew Pinski via
> > > Gcc
> > > Sent: 30 April 2020 07:21
> > > To: Florian Weimer 
> > > Cc: GCC Mailing List ; nmeye...@amzn.com
> > > Subject: Re: Should ARMv8-A generic tuning default to
> > > -moutline-atomics
> > >
> > > On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc
> > > 
> > > wrote:
> > > >
> > > > Distributions are receiving requests to build things with
> > > > -moutline-atomics:
> > > >
> > > >   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
> > > >
> > > > Should this be reflected in the GCC upstream defaults for ARMv8-A
> > > > generic tuning?  It does not make much sense to me if every
> > > > distribution has to overide these flags, either in their build
> > > > system or by patching GCC.
> > >
> > > At least we should make it a configure option.
> > > I do want the ability to default it for our (Marvell) toolchain for
> > > Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> > > anyways).
> >
> > After some internal discussions, I am open to having it on as a default.
> > Here are two versions. One has it as a tuning setting that CPUs can
> > override, the other just switches it on by default always unless
> > overridden by -mno- outline-atomics.
> > I slightly prefer the second one as it's cleaner and simpler, but
> > happy to take either.
> > Any preferences?
> > Thanks,
> > Kyrill
> >
> > ChangeLogs:
> >
> > 2020-04-30  Kyrylo Tkachov  
> >
> > * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics):
> > Declare.
> > * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
> > * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> > variable.
> >
> > 2020-04-30  Kyrylo Tkachov  
> >
> > * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
> > * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> > variable.
> > * doc/invoke.texi (moutline-atomics): Document as on by default.
> >
> 
> I've pushed this second variant after bootstrap and testing on aarch64-none-
> linux-gnu.
> Compiled a simple atomic-using testcase with all relevant combinations of -
> moutline-atomics and LSE and no-LSE -march options.
> Before the results were (as expected):
>  |-moutline-atomics | -mno-outline-atomics |  option
> 
> LSE  |  inline LSE  | inline LSE| inline LSE
> no-LSE   |  outline | inline LDXR/STXR  | inline LDX/STXR
> 
> 
> with this patch they are:
>  -moutline-atomics  | -mno-outline-atomics |  option>
> 
> LSE  |  inline LSE  | inline LSE   | inline LSE
> no-LSE   |  outline | inline LDXR/STXR | outline
> 
> Thanks,
> Kyrill

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Kyrylo Tkachov]

> -Original Message-
> From: Gcc-patches  On Behalf Of Kyrylo
> Tkachov
> Sent: 30 April 2020 11:57
> To: Andrew Pinski ; Florian Weimer
> 
> Cc: gcc-patches@gcc.gnu.org; nmeye...@amzn.com
> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> [Moving to gcc-patches]
> 
> > -Original Message-
> > From: Gcc  On Behalf Of Andrew Pinski via Gcc
> > Sent: 30 April 2020 07:21
> > To: Florian Weimer 
> > Cc: GCC Mailing List ; nmeye...@amzn.com
> > Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> >
> > On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
> > wrote:
> > >
> > > Distributions are receiving requests to build things with
> > > -moutline-atomics:
> > >
> > >   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
> > >
> > > Should this be reflected in the GCC upstream defaults for ARMv8-A
> > > generic tuning?  It does not make much sense to me if every distribution
> > > has to overide these flags, either in their build system or by patching
> > > GCC.
> >
> > At least we should make it a configure option.
> > I do want the ability to default it for our (Marvell) toolchain for
> > Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> > anyways).
> 
> After some internal discussions, I am open to having it on as a default.
> Here are two versions. One has it as a tuning setting that CPUs can override,
> the other just switches it on by default always unless overridden by -mno-
> outline-atomics.
> I slightly prefer the second one as it's cleaner and simpler, but happy to 
> take
> either.
> Any preferences?
> Thanks,
> Kyrill
> 
> ChangeLogs:
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics):
> Declare.
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> variable.
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int
> variable.
>   * doc/invoke.texi (moutline-atomics): Document as on by default.
> 

I've pushed this second variant after bootstrap and testing on 
aarch64-none-linux-gnu.
Compiled a simple atomic-using testcase with all relevant combinations of 
-moutline-atomics and LSE and no-LSE -march options.
Before the results were (as expected):
 |-moutline-atomics | -mno-outline-atomics | 

LSE  |  inline LSE  | inline LSE   | inline LSE
no-LSE   |  outline | inline LDXR/STXR | outline

Thanks,
Kyrill

Re: Should ARMv8-A generic tuning default to -moutline-atomics

[Richard Earnshaw]

On 30/04/2020 11:56, Kyrylo Tkachov wrote:
> [Moving to gcc-patches]
> 
>> -Original Message-
>> From: Gcc  On Behalf Of Andrew Pinski via Gcc
>> Sent: 30 April 2020 07:21
>> To: Florian Weimer 
>> Cc: GCC Mailing List ; nmeye...@amzn.com
>> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
>>
>> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
>> wrote:
>>>
>>> Distributions are receiving requests to build things with
>>> -moutline-atomics:
>>>
>>>   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
>>>
>>> Should this be reflected in the GCC upstream defaults for ARMv8-A
>>> generic tuning?  It does not make much sense to me if every distribution
>>> has to overide these flags, either in their build system or by patching
>>> GCC.
>>
>> At least we should make it a configure option.
>> I do want the ability to default it for our (Marvell) toolchain for
>> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
>> anyways).
> 
> After some internal discussions, I am open to having it on as a default.
> Here are two versions. One has it as a tuning setting that CPUs can override, 
> the other just switches it on by default always unless overridden by 
> -mno-outline-atomics.
> I slightly prefer the second one as it's cleaner and simpler, but happy to 
> take either.
> Any preferences?
> Thanks,
> Kyrill
> 
> ChangeLogs:
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64-tuning-flags.def (no_outline_atomics): Declare.
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.
> 
> 2020-04-30  Kyrylo Tkachov  
> 
>   * config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
>   * config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.
>   * doc/invoke.texi (moutline-atomics): Document as on by default.
> 
> 

I think I prefer the second option.

The whole point of LSE (and hence the name "Large System Extension") was
due to the fact that when you have many cores the v8.0 atomics have
known scaling issues.  It's not a property of the core though, it's
primarily a property of the number of cores in the system.  The problem
really is that we don't have a tuning param for that, nor can we really
tell at compile time how many threads might really be in use.

So I don't think this is really a tuning param that can/should be
selected via the existing -mtune tables.

R.

RE: Should ARMv8-A generic tuning default to -moutline-atomics

[Kyrylo Tkachov]

[Moving to gcc-patches]

> -Original Message-
> From: Gcc  On Behalf Of Andrew Pinski via Gcc
> Sent: 30 April 2020 07:21
> To: Florian Weimer 
> Cc: GCC Mailing List ; nmeye...@amzn.com
> Subject: Re: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc 
> wrote:
> >
> > Distributions are receiving requests to build things with
> > -moutline-atomics:
> >
> >   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956418>
> >
> > Should this be reflected in the GCC upstream defaults for ARMv8-A
> > generic tuning?  It does not make much sense to me if every distribution
> > has to overide these flags, either in their build system or by patching
> > GCC.
> 
> At least we should make it a configure option.
> I do want the ability to default it for our (Marvell) toolchain for
> Linux (our bare metal toolchain will be defaulting to ARMv8.2-a
> anyways).

After some internal discussions, I am open to having it on as a default.
Here are two versions. One has it as a tuning setting that CPUs can override, 
the other just switches it on by default always unless overridden by 
-mno-outline-atomics.
I slightly prefer the second one as it's cleaner and simpler, but happy to take 
either.
Any preferences?
Thanks,
Kyrill

ChangeLogs:

2020-04-30  Kyrylo Tkachov  

* config/aarch64/aarch64-tuning-flags.def (no_outline_atomics): Declare.
* config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
* config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.

2020-04-30  Kyrylo Tkachov  

* config/aarch64/aarch64.h (TARGET_OUTLINE_ATOMICS): Define.
* config/aarch64/aarch64.opt (moutline-atomics): Change to Int variable.
* doc/invoke.texi (moutline-atomics): Document as on by default.




ool-default-tune.patch
Description: ool-default-tune.patch


ool-default-no-tune.patch
Description: ool-default-no-tune.patch