Re: [PATCH] Add outline-atomics to target attribute.
Martin Liška writes: > On 5/21/20 12:11 PM, Richard Sandiford wrote: >> Yeah, agree it'd be worth having tests for both directions. The patch >> itself looks good though -- thanks for doing this. > > Thanks. There's a version with 2 new tests that I've just tested. > > I'm going to install the patch for master. Is it also fine for all > active branches where you backported the atomics outlining? Yeah, OK for all those too. Thanks, Richard > Martin > > From 357a86fbf18ad511a72e6e9c703a6e0b0dd30ddf Mon Sep 17 00:00:00 2001 > From: Martin Liska > Date: Thu, 21 May 2020 09:19:21 +0200 > Subject: [PATCH] Add outline-atomics to target attribute. > > gcc/ChangeLog: > > 2020-05-21 Martin Liska > > * common/config/aarch64/aarch64-common.c (aarch64_handle_option): > Handle OPT_moutline_atomics. > * config/aarch64/aarch64.c: Add outline-atomics to > aarch64_attributes. > * doc/extend.texi: Document the newly added target attribute. > > gcc/testsuite/ChangeLog: > > 2020-05-21 Martin Liska > > * gcc.target/aarch64/target_attr_20.c: New test. > * gcc.target/aarch64/target_attr_21.c: New test. > --- > gcc/common/config/aarch64/aarch64-common.c| 4 +++ > gcc/config/aarch64/aarch64.c | 2 ++ > gcc/doc/extend.texi | 6 + > .../gcc.target/aarch64/target_attr_20.c | 27 +++ > .../gcc.target/aarch64/target_attr_21.c | 27 +++ > 5 files changed, 66 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/aarch64/target_attr_20.c > create mode 100644 gcc/testsuite/gcc.target/aarch64/target_attr_21.c > > diff --git a/gcc/common/config/aarch64/aarch64-common.c > b/gcc/common/config/aarch64/aarch64-common.c > index 0bddcc8c3e9..51bd319d6d3 100644 > --- a/gcc/common/config/aarch64/aarch64-common.c > +++ b/gcc/common/config/aarch64/aarch64-common.c > @@ -116,6 +116,10 @@ aarch64_handle_option (struct gcc_options *opts, >opts->x_flag_omit_leaf_frame_pointer = val; >return true; > > +case OPT_moutline_atomics: > + opts->x_aarch64_flag_outline_atomics = val; > + return true; > + > default: >return true; > } > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 79c016f4dc3..78db0a56323 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -15266,6 +15266,8 @@ static const struct aarch64_attribute_info > aarch64_attributes[] = > aarch64_handle_attr_branch_protection, OPT_mbranch_protection_ }, >{ "sign-return-address", aarch64_attr_enum, false, NULL, > OPT_msign_return_address_ }, > + { "outline-atomics", aarch64_attr_bool, true, NULL, > + OPT_moutline_atomics}, >{ NULL, aarch64_attr_custom, false, NULL, OPT } > }; > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index c80848e9061..a2ebef8cf8c 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -4066,6 +4066,12 @@ Select the function scope on which branch protection > will be applied. The > behavior and permissible arguments are the same as for the command-line > option > @option{-mbranch-protection=}. The default value is @code{none}. > > +@item outline-atomics > +@cindex @code{outline-atomics} function attribute, AArch64 > +Enable or disable calls to out-of-line helpers to implement atomic > operations. > +This corresponds to the behavior of the command line options > +@option{-moutline-atomics} and @option{-mno-outline-atomics}. > + > @end table > > The above target attributes can be specified as follows: > diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_20.c > b/gcc/testsuite/gcc.target/aarch64/target_attr_20.c > new file mode 100644 > index 000..509fb039e84 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_20.c > @@ -0,0 +1,27 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=armv8-a+nolse -moutline-atomics" } */ > + > +int b, c, d, e, f, h; > +short g; > +int foo (int) __attribute__ ((__const__)); > + > +__attribute__ ((target ("no-outline-atomics"))) > +void > +bar (void) > +{ > + while (1) > +{ > + while (1) > + { > + __atomic_load_n (, 0); > + if (foo (2)) > + __sync_val_compare_and_swap (, 0, f); > + b = 1; > + if (h == e) > + break; > + } > + __sync_val_compare_and_swap (, -1, f); > +} > +} > + > +/* { dg-final { scan-assembler-not "bl.*__aarch64_cas2_acq_rel" } } */ > diff --git a/gcc/testsuite/gc
Re: [PATCH] Add outline-atomics to target attribute.
On 5/21/20 12:11 PM, Richard Sandiford wrote: Yeah, agree it'd be worth having tests for both directions. The patch itself looks good though -- thanks for doing this. Thanks. There's a version with 2 new tests that I've just tested. I'm going to install the patch for master. Is it also fine for all active branches where you backported the atomics outlining? Martin >From 357a86fbf18ad511a72e6e9c703a6e0b0dd30ddf Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 21 May 2020 09:19:21 +0200 Subject: [PATCH] Add outline-atomics to target attribute. gcc/ChangeLog: 2020-05-21 Martin Liska * common/config/aarch64/aarch64-common.c (aarch64_handle_option): Handle OPT_moutline_atomics. * config/aarch64/aarch64.c: Add outline-atomics to aarch64_attributes. * doc/extend.texi: Document the newly added target attribute. gcc/testsuite/ChangeLog: 2020-05-21 Martin Liska * gcc.target/aarch64/target_attr_20.c: New test. * gcc.target/aarch64/target_attr_21.c: New test. --- gcc/common/config/aarch64/aarch64-common.c| 4 +++ gcc/config/aarch64/aarch64.c | 2 ++ gcc/doc/extend.texi | 6 + .../gcc.target/aarch64/target_attr_20.c | 27 +++ .../gcc.target/aarch64/target_attr_21.c | 27 +++ 5 files changed, 66 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/target_attr_20.c create mode 100644 gcc/testsuite/gcc.target/aarch64/target_attr_21.c diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c index 0bddcc8c3e9..51bd319d6d3 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -116,6 +116,10 @@ aarch64_handle_option (struct gcc_options *opts, opts->x_flag_omit_leaf_frame_pointer = val; return true; +case OPT_moutline_atomics: + opts->x_aarch64_flag_outline_atomics = val; + return true; + default: return true; } diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 79c016f4dc3..78db0a56323 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -15266,6 +15266,8 @@ static const struct aarch64_attribute_info aarch64_attributes[] = aarch64_handle_attr_branch_protection, OPT_mbranch_protection_ }, { "sign-return-address", aarch64_attr_enum, false, NULL, OPT_msign_return_address_ }, + { "outline-atomics", aarch64_attr_bool, true, NULL, + OPT_moutline_atomics}, { NULL, aarch64_attr_custom, false, NULL, OPT } }; diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index c80848e9061..a2ebef8cf8c 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -4066,6 +4066,12 @@ Select the function scope on which branch protection will be applied. The behavior and permissible arguments are the same as for the command-line option @option{-mbranch-protection=}. The default value is @code{none}. +@item outline-atomics +@cindex @code{outline-atomics} function attribute, AArch64 +Enable or disable calls to out-of-line helpers to implement atomic operations. +This corresponds to the behavior of the command line options +@option{-moutline-atomics} and @option{-mno-outline-atomics}. + @end table The above target attributes can be specified as follows: diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_20.c b/gcc/testsuite/gcc.target/aarch64/target_attr_20.c new file mode 100644 index 000..509fb039e84 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_20.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-march=armv8-a+nolse -moutline-atomics" } */ + +int b, c, d, e, f, h; +short g; +int foo (int) __attribute__ ((__const__)); + +__attribute__ ((target ("no-outline-atomics"))) +void +bar (void) +{ + while (1) +{ + while (1) + { + __atomic_load_n (, 0); + if (foo (2)) + __sync_val_compare_and_swap (, 0, f); + b = 1; + if (h == e) + break; + } + __sync_val_compare_and_swap (, -1, f); +} +} + +/* { dg-final { scan-assembler-not "bl.*__aarch64_cas2_acq_rel" } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_21.c b/gcc/testsuite/gcc.target/aarch64/target_attr_21.c new file mode 100644 index 000..acace4c8f2a --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_21.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-march=armv8-a+nolse -mno-outline-atomics" } */ + +int b, c, d, e, f, h; +short g; +int foo (int) __attribute__ ((__const__)); + +__attribute__ ((target ("outline-atomics"))) +void +bar (void) +{ + while (1) +{ + while (1) + { + __atomic_load_n (, 0); + if (foo (2)) + __sync_val_compare_and_swap (, 0, f); + b = 1; + if (h == e) + break; + } + __sync_val_compare_and_swap (, -1, f); +} +} + +/* { dg-final { scan-assembler-times "bl.*__aarch64_cas2_acq_rel" 1 } } */ -- 2.26.2
Re: [PATCH] Add outline-atomics to target attribute.
Jakub Jelinek via Gcc-patches writes: > On Thu, May 21, 2020 at 10:03:37AM +0200, Martin Liška wrote: >> On 5/21/20 9:56 AM, Jakub Jelinek wrote: >> > Can't it be __attribute__((target ("outline-atomics"))) instead? >> >> Ah sorry, I wan unclear. >> It's support the this _target_ attribute which you mentioned. > > Ok, better, will defer review to target maintainers of course. > But the above clearly shows you forgot to add testcase coverage, > both that dg-options -moutline-atomics target "no-outline-atomics" > routine with scan-assembler and the other way around. Yeah, agree it'd be worth having tests for both directions. The patch itself looks good though -- thanks for doing this. Richard
Re: [PATCH] Add outline-atomics to target attribute.
On Thu, May 21, 2020 at 10:03:37AM +0200, Martin Liška wrote: > On 5/21/20 9:56 AM, Jakub Jelinek wrote: > > Can't it be __attribute__((target ("outline-atomics"))) instead? > > Ah sorry, I wan unclear. > It's support the this _target_ attribute which you mentioned. Ok, better, will defer review to target maintainers of course. But the above clearly shows you forgot to add testcase coverage, both that dg-options -moutline-atomics target "no-outline-atomics" routine with scan-assembler and the other way around. Jakub
Re: [PATCH] Add outline-atomics to target attribute.
On 5/21/20 9:56 AM, Jakub Jelinek wrote: Can't it be __attribute__((target ("outline-atomics"))) instead? Ah sorry, I wan unclear. It's support the this _target_ attribute which you mentioned. Martin
Re: [PATCH] Add outline-atomics to target attribute.
On Thu, May 21, 2020 at 09:23:47AM +0200, Martin Liška wrote: > As mentioned in the ovmf project [1], they would like to have a control > over the -mno-outline-atomics via a function attribute. They struggle > with configure detection and this can help them to disable the outlining. > > Would it be possible to add the attribute? Can't it be __attribute__((target ("outline-atomics"))) instead? I'm not a fan of adding new attributes that just stand for enable this or disable this command line option for a particular function, we have target and optimize attributes for that. Jakub
[PATCH] Add outline-atomics to target attribute.
Hi. As mentioned in the ovmf project [1], they would like to have a control over the -mno-outline-atomics via a function attribute. They struggle with configure detection and this can help them to disable the outlining. Would it be possible to add the attribute? Thanks, Martin [1] https://bugzilla.tianocore.org/show_bug.cgi?id=2723 gcc/ChangeLog: 2020-05-21 Martin Liska * common/config/aarch64/aarch64-common.c (aarch64_handle_option): Handle OPT_moutline_atomics. * config/aarch64/aarch64.c: Add outline-atomics to aarch64_attributes. * doc/extend.texi: Document the newly added target attribute. --- gcc/common/config/aarch64/aarch64-common.c | 4 gcc/config/aarch64/aarch64.c | 2 ++ gcc/doc/extend.texi| 6 ++ 3 files changed, 12 insertions(+) diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c index 0bddcc8c3e9..51bd319d6d3 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -116,6 +116,10 @@ aarch64_handle_option (struct gcc_options *opts, opts->x_flag_omit_leaf_frame_pointer = val; return true; +case OPT_moutline_atomics: + opts->x_aarch64_flag_outline_atomics = val; + return true; + default: return true; } diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 79c016f4dc3..78db0a56323 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -15266,6 +15266,8 @@ static const struct aarch64_attribute_info aarch64_attributes[] = aarch64_handle_attr_branch_protection, OPT_mbranch_protection_ }, { "sign-return-address", aarch64_attr_enum, false, NULL, OPT_msign_return_address_ }, + { "outline-atomics", aarch64_attr_bool, true, NULL, + OPT_moutline_atomics}, { NULL, aarch64_attr_custom, false, NULL, OPT } }; diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index c80848e9061..a2ebef8cf8c 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -4066,6 +4066,12 @@ Select the function scope on which branch protection will be applied. The behavior and permissible arguments are the same as for the command-line option @option{-mbranch-protection=}. The default value is @code{none}. +@item outline-atomics +@cindex @code{outline-atomics} function attribute, AArch64 +Enable or disable calls to out-of-line helpers to implement atomic operations. +This corresponds to the behavior of the command line options +@option{-moutline-atomics} and @option{-mno-outline-atomics}. + @end table The above target attributes can be specified as follows: