Add outline-atomics to target attribute.

Message ID 0961768a-a5a7-e6f8-8c48-0c1e27709920@suse.cz
State New
Headers show
Series
  • Add outline-atomics to target attribute.
Related show

Commit Message

Martin Liška May 21, 2020, 7:23 a.m.
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  <mliska@suse.cz>

	* 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(+)

Comments

xionghu luo via Gcc-patches May 21, 2020, 7:56 a.m. | #1
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
Martin Liška May 21, 2020, 8:03 a.m. | #2
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
xionghu luo via Gcc-patches May 21, 2020, 8:44 a.m. | #3
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
Richard Sandiford May 21, 2020, 10:11 a.m. | #4
Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> 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
Martin Liška May 21, 2020, 10:40 a.m. | #5
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 <mliska@suse.cz>

Date: Thu, 21 May 2020 09:19:21 +0200
Subject: [PATCH] Add outline-atomics to target attribute.

gcc/ChangeLog:

2020-05-21  Martin Liska  <mliska@suse.cz>

	* 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  <mliska@suse.cz>

	* 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 00000000000..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 (&e, 0);
+	  if (foo (2))
+	    __sync_val_compare_and_swap (&c, 0, f);
+	  b = 1;
+	  if (h == e)
+	    break;
+	}
+      __sync_val_compare_and_swap (&g, -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 00000000000..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 (&e, 0);
+	  if (foo (2))
+	    __sync_val_compare_and_swap (&c, 0, f);
+	  b = 1;
+	  if (h == e)
+	    break;
+	}
+      __sync_val_compare_and_swap (&g, -1, f);
+    }
+}
+
+/* { dg-final { scan-assembler-times "bl.*__aarch64_cas2_acq_rel" 1 } } */
-- 
2.26.2
Richard Sandiford May 21, 2020, 2:19 p.m. | #6
Martin Liška <mliska@suse.cz> 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 <mliska@suse.cz>

> Date: Thu, 21 May 2020 09:19:21 +0200

> Subject: [PATCH] Add outline-atomics to target attribute.

>

> gcc/ChangeLog:

>

> 2020-05-21  Martin Liska  <mliska@suse.cz>

>

> 	* 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  <mliska@suse.cz>

>

> 	* 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 00000000000..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 (&e, 0);

> +	  if (foo (2))

> +	    __sync_val_compare_and_swap (&c, 0, f);

> +	  b = 1;

> +	  if (h == e)

> +	    break;

> +	}

> +      __sync_val_compare_and_swap (&g, -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 00000000000..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 (&e, 0);

> +	  if (foo (2))

> +	    __sync_val_compare_and_swap (&c, 0, f);

> +	  b = 1;

> +	  if (h == e)

> +	    break;

> +	}

> +      __sync_val_compare_and_swap (&g, -1, f);

> +    }

> +}

> +

> +/* { dg-final { scan-assembler-times "bl.*__aarch64_cas2_acq_rel" 1 } } */

Patch

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: