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

2020-05-21 Thread Richard Sandiford
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.

2020-05-21 Thread Martin Liška

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.

2020-05-21 Thread Richard Sandiford
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.

2020-05-21 Thread Jakub Jelinek via Gcc-patches
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.

2020-05-21 Thread Martin Liška

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.

2020-05-21 Thread Jakub Jelinek via Gcc-patches
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.

2020-05-21 Thread Martin Liška

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: