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

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

[Kyrylo Tkachov]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

[Moving to gcc-patches]

> -----Original Message-----
> From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via Gcc
> Sent: 30 April 2020 07:21
> To: Florian Weimer <fweimer@redhat.com>
> Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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 <gcc@gcc.gnu.org>
> 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  <kyrylo.tkachov@arm.com>

	* 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  <kyrylo.tkachov@arm.com>

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



[-- Attachment #2: ool-default-tune.patch --]
[-- Type: application/octet-stream, Size: 2049 bytes --]

diff --git a/gcc/config/aarch64/aarch64-tuning-flags.def b/gcc/config/aarch64/aarch64-tuning-flags.def
index ccef3c09273..cc574b41fb6 100644
--- a/gcc/config/aarch64/aarch64-tuning-flags.def
+++ b/gcc/config/aarch64/aarch64-tuning-flags.def
@@ -46,4 +46,8 @@ AARCH64_EXTRA_TUNING_OPTION ("no_ldp_stp_qregs", NO_LDP_STP_QREGS)
 
 AARCH64_EXTRA_TUNING_OPTION ("rename_load_regs", RENAME_LOAD_REGS)
 
+/* Do not emit outlined libgcc helper calls for atomic operations
+   for non-LSE targets.  */
+AARCH64_EXTRA_TUNING_OPTION ("no_outline_atomics", NO_OUTLINE_ATOMICS)
+
 #undef AARCH64_EXTRA_TUNING_OPTION
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 74236c3cffd..695d56c50ef 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -105,6 +105,17 @@
    port.  */
 #define TARGET_PTRMEMFUNC_VBIT_LOCATION ptrmemfunc_vbit_in_delta
 
+
+/* Emit calls to libgcc helpers for atomic operations for runtime detection
+   of LSE instructions.  Use if explicitly asked for by the option.
+   Alternatively, use it by default unless CPU tuning explicitly disables
+   it.  */
+#define TARGET_OUTLINE_ATOMICS					\
+  (aarch64_flag_outline_atomics == 1				\
+    || (aarch64_flag_outline_atomics == 2			\
+	  && ((aarch64_tune_params.extra_tuning_flags		\
+	      & AARCH64_EXTRA_TUNE_NO_OUTLINE_ATOMICS) == 0)))
+
 /* Align definitions of arrays, unions and structures so that
    initializations and copies can be made more efficient.  This is not
    ABI-changing, so it only affects places where we can see the
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 37181b5baca..d99d14c137d 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -256,7 +256,7 @@ TargetVariable
 long aarch64_stack_protector_guard_offset = 0
 
 moutline-atomics
-Target Report Mask(OUTLINE_ATOMICS) Save
+Target Report Var(aarch64_flag_outline_atomics) Init(2) Save
 Generate local calls to out-of-line atomic operations.
 
 -param=aarch64-sve-compare-costs=

[-- Attachment #3: ool-default-no-tune.patch --]
[-- Type: application/octet-stream, Size: 1655 bytes --]

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 74236c3cffd..24767c747ba 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -105,6 +105,11 @@
    port.  */
 #define TARGET_PTRMEMFUNC_VBIT_LOCATION ptrmemfunc_vbit_in_delta
 
+
+/* Emit calls to libgcc helpers for atomic operations for runtime detection
+   of LSE instructions.  */
+#define TARGET_OUTLINE_ATOMICS (aarch64_flag_outline_atomics)
+
 /* Align definitions of arrays, unions and structures so that
    initializations and copies can be made more efficient.  This is not
    ABI-changing, so it only affects places where we can see the
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 37181b5baca..d99d14c137d 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -256,7 +256,7 @@ TargetVariable
 long aarch64_stack_protector_guard_offset = 0
 
 moutline-atomics
-Target Report Mask(OUTLINE_ATOMICS) Save
+Target Report Var(aarch64_flag_outline_atomics) Init(2) Save
 Generate local calls to out-of-line atomic operations.
 
 -param=aarch64-sve-compare-costs=
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5bb7d94833e..527d362533a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16886,6 +16886,7 @@ instruction set.  If using a later revision, e.g. @option{-march=armv8.1-a}
 or @option{-march=armv8-a+lse}, the ARMv8.1-Atomics instructions will be
 used directly.  The same applies when using @option{-mcpu=} when the
 selected cpu supports the @samp{lse} feature.
+This option is on by default.
 
 @item -march=@var{name}
 @opindex march

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 <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via Gcc
>> Sent: 30 April 2020 07:21
>> To: Florian Weimer <fweimer@redhat.com>
>> Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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 <gcc@gcc.gnu.org>
>> 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  <kyrylo.tkachov@arm.com>
> 
> 	* 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  <kyrylo.tkachov@arm.com>
> 
> 	* 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]

> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of Kyrylo
> Tkachov
> Sent: 30 April 2020 11:57
> To: Andrew Pinski <pinskia@gmail.com>; Florian Weimer
> <fweimer@redhat.com>
> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> [Moving to gcc-patches]
> 
> > -----Original Message-----
> > From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via Gcc
> > Sent: 30 April 2020 07:21
> > To: Florian Weimer <fweimer@redhat.com>
> > Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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 <gcc@gcc.gnu.org>
> > 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  <kyrylo.tkachov@arm.com>
> 
> 	* 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  <kyrylo.tkachov@arm.com>
> 
> 	* 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 | <no 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 | <no 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]

Hi Kyrill,

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

Thanks,
-Jiangning

> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of Kyrylo
> Tkachov
> Sent: Thursday, April 30, 2020 8:27 PM
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Andrew Pinski
> <pinskia@gmail.com>; Florian Weimer <fweimer@redhat.com>
> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
> 
> 
> 
> > -----Original Message-----
> > From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
> > Kyrylo Tkachov
> > Sent: 30 April 2020 11:57
> > To: Andrew Pinski <pinskia@gmail.com>; Florian Weimer
> > <fweimer@redhat.com>
> > Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> > Subject: RE: Should ARMv8-A generic tuning default to
> > -moutline-atomics
> >
> > [Moving to gcc-patches]
> >
> > > -----Original Message-----
> > > From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via
> > > Gcc
> > > Sent: 30 April 2020 07:21
> > > To: Florian Weimer <fweimer@redhat.com>
> > > Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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
> > > <gcc@gcc.gnu.org>
> > > 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  <kyrylo.tkachov@arm.com>
> >
> > 	* 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  <kyrylo.tkachov@arm.com>
> >
> > 	* 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 | <no 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 | <no 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 <gcc-patches-bounces@gcc.gnu.org> On Behalf Of Kyrylo
>> Tkachov
>> Sent: Thursday, April 30, 2020 8:27 PM
>> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Andrew Pinski
>> <pinskia@gmail.com>; Florian Weimer <fweimer@redhat.com>
>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
>> Subject: RE: Should ARMv8-A generic tuning default to -moutline-atomics
>>
>>
>>
>>> -----Original Message-----
>>> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
>>> Kyrylo Tkachov
>>> Sent: 30 April 2020 11:57
>>> To: Andrew Pinski <pinskia@gmail.com>; Florian Weimer
>>> <fweimer@redhat.com>
>>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
>>> Subject: RE: Should ARMv8-A generic tuning default to
>>> -moutline-atomics
>>>
>>> [Moving to gcc-patches]
>>>
>>>> -----Original Message-----
>>>> From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via
>>>> Gcc
>>>> Sent: 30 April 2020 07:21
>>>> To: Florian Weimer <fweimer@redhat.com>
>>>> Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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
>>>> <gcc@gcc.gnu.org>
>>>> 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  <kyrylo.tkachov@arm.com>
>>>
>>> 	* 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  <kyrylo.tkachov@arm.com>
>>>
>>> 	* 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 | <no 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 | <no 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]

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 <Richard.Earnshaw@foss.arm.com>
> Sent: Friday, May 1, 2020 6:41 PM
> To: JiangNing OS <jiangning@os.amperecomputing.com>; Kyrylo Tkachov
> <Kyrylo.Tkachov@arm.com>; Andrew Pinski <pinskia@gmail.com>; Florian
> Weimer <fweimer@redhat.com>
> Cc: gcc-patches@gcc.gnu.org; nmeyerha@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 <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
> >> Kyrylo Tkachov
> >> Sent: Thursday, April 30, 2020 8:27 PM
> >> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Andrew Pinski
> >> <pinskia@gmail.com>; Florian Weimer <fweimer@redhat.com>
> >> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> >> Subject: RE: Should ARMv8-A generic tuning default to
> >> -moutline-atomics
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
> >>> Kyrylo Tkachov
> >>> Sent: 30 April 2020 11:57
> >>> To: Andrew Pinski <pinskia@gmail.com>; Florian Weimer
> >>> <fweimer@redhat.com>
> >>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> >>> Subject: RE: Should ARMv8-A generic tuning default to
> >>> -moutline-atomics
> >>>
> >>> [Moving to gcc-patches]
> >>>
> >>>> -----Original Message-----
> >>>> From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via
> >>>> Gcc
> >>>> Sent: 30 April 2020 07:21
> >>>> To: Florian Weimer <fweimer@redhat.com>
> >>>> Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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
> >>>> <gcc@gcc.gnu.org>
> >>>> 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  <kyrylo.tkachov@arm.com>
> >>>
> >>> 	* 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  <kyrylo.tkachov@arm.com>
> >>>
> >>> 	* 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 | <no
> >> 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 | <no
> >> 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]

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

> -----Original Message-----
> From: JiangNing OS <jiangning@os.amperecomputing.com>
> Sent: 01 May 2020 11:49
> To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>; Kyrylo Tkachov
> <Kyrylo.Tkachov@arm.com>; Andrew Pinski <pinskia@gmail.com>; Florian
> Weimer <fweimer@redhat.com>
> Cc: gcc-patches@gcc.gnu.org; nmeyerha@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 <Richard.Earnshaw@foss.arm.com>
> > Sent: Friday, May 1, 2020 6:41 PM
> > To: JiangNing OS <jiangning@os.amperecomputing.com>; Kyrylo Tkachov
> > <Kyrylo.Tkachov@arm.com>; Andrew Pinski <pinskia@gmail.com>; Florian
> > Weimer <fweimer@redhat.com>
> > Cc: gcc-patches@gcc.gnu.org; nmeyerha@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 <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
> > >> Kyrylo Tkachov
> > >> Sent: Thursday, April 30, 2020 8:27 PM
> > >> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Andrew Pinski
> > >> <pinskia@gmail.com>; Florian Weimer <fweimer@redhat.com>
> > >> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> > >> Subject: RE: Should ARMv8-A generic tuning default to
> > >> -moutline-atomics
> > >>
> > >>
> > >>
> > >>> -----Original Message-----
> > >>> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
> > >>> Kyrylo Tkachov
> > >>> Sent: 30 April 2020 11:57
> > >>> To: Andrew Pinski <pinskia@gmail.com>; Florian Weimer
> > >>> <fweimer@redhat.com>
> > >>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
> > >>> Subject: RE: Should ARMv8-A generic tuning default to
> > >>> -moutline-atomics
> > >>>
> > >>> [Moving to gcc-patches]
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski
> via
> > >>>> Gcc
> > >>>> Sent: 30 April 2020 07:21
> > >>>> To: Florian Weimer <fweimer@redhat.com>
> > >>>> Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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
> > >>>> <gcc@gcc.gnu.org>
> > >>>> 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  <kyrylo.tkachov@arm.com>
> > >>>
> > >>> 	* 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  <kyrylo.tkachov@arm.com>
> > >>>
> > >>> 	* 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 | <no
> > >> 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 | <no
> > >> 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 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 <jiangning@os.amperecomputing.com>
>> Sent: 01 May 2020 11:49
>> To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>; Kyrylo Tkachov
>> <Kyrylo.Tkachov@arm.com>; Andrew Pinski <pinskia@gmail.com>; Florian
>> Weimer <fweimer@redhat.com>
>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@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 <Richard.Earnshaw@foss.arm.com>
>>> Sent: Friday, May 1, 2020 6:41 PM
>>> To: JiangNing OS <jiangning@os.amperecomputing.com>; Kyrylo Tkachov
>>> <Kyrylo.Tkachov@arm.com>; Andrew Pinski <pinskia@gmail.com>; Florian
>>> Weimer <fweimer@redhat.com>
>>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@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 <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
>>>>> Kyrylo Tkachov
>>>>> Sent: Thursday, April 30, 2020 8:27 PM
>>>>> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Andrew Pinski
>>>>> <pinskia@gmail.com>; Florian Weimer <fweimer@redhat.com>
>>>>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
>>>>> Subject: RE: Should ARMv8-A generic tuning default to
>>>>> -moutline-atomics
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
>>>>>> Kyrylo Tkachov
>>>>>> Sent: 30 April 2020 11:57
>>>>>> To: Andrew Pinski <pinskia@gmail.com>; Florian Weimer
>>>>>> <fweimer@redhat.com>
>>>>>> Cc: gcc-patches@gcc.gnu.org; nmeyerha@amzn.com
>>>>>> Subject: RE: Should ARMv8-A generic tuning default to
>>>>>> -moutline-atomics
>>>>>>
>>>>>> [Moving to gcc-patches]
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski
>> via
>>>>>>> Gcc
>>>>>>> Sent: 30 April 2020 07:21
>>>>>>> To: Florian Weimer <fweimer@redhat.com>
>>>>>>> Cc: GCC Mailing List <gcc@gcc.gnu.org>; nmeyerha@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
>>>>>>> <gcc@gcc.gnu.org>
>>>>>>> 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  <kyrylo.tkachov@arm.com>
>>>>>>
>>>>>> 	* 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  <kyrylo.tkachov@arm.com>
>>>>>>
>>>>>> 	* 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 | <no
>>>>> 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 | <no
>>>>> 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

[Jeff Law]

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 <jiangning@os.amperecomputing.com>
> > > Sent: 01 May 2020 11:49
> > > To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>; Kyrylo Tkachov
> > > <Kyrylo.Tkachov@arm.com>; Andrew Pinski <pinskia@gmail.com>; Florian
> > > Weimer <fweimer@redhat.com>
> > > Cc: gcc-patches@gcc.gnu.org; nmeyerha@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

[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
joseph@codesourcery.com

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

[Florian Weimer]

* 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

[Kyrylo Tkachov]

Hi all,

> -----Original Message-----
> From: Florian Weimer <fweimer@redhat.com>
> Sent: 04 May 2020 09:35
> To: Joseph Myers <joseph@codesourcery.com>
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Andrew Pinski
> <pinskia@gmail.com>; gcc-patches@gcc.gnu.org; nmeyerha@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

[Andrew Haley]

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://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

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

[Szabolcs Nagy]

The 04/30/2020 12:26, Kyrylo Tkachov wrote:
> > > From: Gcc <gcc-bounces@gcc.gnu.org> On Behalf Of Andrew Pinski via Gcc
> > > On Wed, Apr 29, 2020 at 6:25 AM Florian Weimer via Gcc <gcc@gcc.gnu.org>
> > > 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  <kyrylo.tkachov@arm.com>
> > 
> > 	* 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  <kyrylo.tkachov@arm.com>
> > 
> > 	* 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 | <no 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 | <no 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