diff mbox series

[AARCH64] Add support for +profile extension

Message ID 5B436135.1090401@arm.com
State New
Headers show
Series [AARCH64] Add support for +profile extension | expand

Commit Message

Andre Vieira (lists) July 9, 2018, 1:20 p.m. UTC 
Hi,

This patch adds support for the Statistical Profiling Extension (SPE) on
AArch64. Even though the compiler will not generate code any differently
given this extension, it will need to pass it on to the assembler in
order to let it correctly assemble inline asm containing accesses to the
extension's system registers.  The same applies when using the
preprocessor on an assembly file as this first must pass through cc1.

I left the hwcaps string for SPE empty as the kernel does not define a
feature string for this extension.  The current effect of this is that
driver will disable profile feature bit in GCC.  This is OK though
because we don't, nor do we ever, enable this feature bit, as codegen is
not affect by the SPE support and more importantly the driver will still
pass the extension down to the assembler regardless.

Boostrapped aarch64-none-linux-gnu and ran regression tests.

Is it OK for trunk?

gcc/ChangeLog:
2018-07-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	* config/aarch64/aarch64-option-extensions.def: New entry for profile
	extension.
	* config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
	* doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
	extension.

gcc/testsuite/ChangeLog:
2018-07-09 Andre Vieira <andre.simoesdiasvieira@arm.com>

	* gcc.target/aarch64/profile.c: New test.

Comments

Kyrill Tkachov July 9, 2018, 1:45 p.m. UTC | #1 
Hi Andre,

On 09/07/18 14:20, Andre Vieira (lists) wrote:
> Hi,
>
> This patch adds support for the Statistical Profiling Extension (SPE) on
> AArch64. Even though the compiler will not generate code any differently
> given this extension, it will need to pass it on to the assembler in
> order to let it correctly assemble inline asm containing accesses to the
> extension's system registers.  The same applies when using the
> preprocessor on an assembly file as this first must pass through cc1.
>
> I left the hwcaps string for SPE empty as the kernel does not define a
> feature string for this extension.  The current effect of this is that
> driver will disable profile feature bit in GCC.  This is OK though
> because we don't, nor do we ever, enable this feature bit, as codegen is
> not affect by the SPE support and more importantly the driver will still
> pass the extension down to the assembler regardless.
>
> Boostrapped aarch64-none-linux-gnu and ran regression tests.
>
> Is it OK for trunk?
>

This looks sensible to me (though you'll need approval from a maintainer) with a small ChangeLog nit...

> gcc/ChangeLog:
> 2018-07-09  Andre Vieira <andre.simoesdiasvieira@arm.com>
>
>         * config/aarch64/aarch64-option-extensions.def: New entry for profile
>         extension.
>         * config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
>         * doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
>         extension.
>
> gcc/testsuite/ChangeLog:
> 2018-07-09 Andre Vieira <andre.simoesdiasvieira@arm.com>
>

... Make sure there's two spaces between the date, name and email.

Thanks,
Kyrill

>         * gcc.target/aarch64/profile.c: New test.
Andrew Pinski July 9, 2018, 4:06 p.m. UTC | #2 
On Mon, Jul 9, 2018 at 6:21 AM Andre Vieira (lists)
<Andre.SimoesDiasVieira@arm.com> wrote:
>
> Hi,
>
> This patch adds support for the Statistical Profiling Extension (SPE) on
> AArch64. Even though the compiler will not generate code any differently
> given this extension, it will need to pass it on to the assembler in
> order to let it correctly assemble inline asm containing accesses to the
> extension's system registers.  The same applies when using the
> preprocessor on an assembly file as this first must pass through cc1.
>
> I left the hwcaps string for SPE empty as the kernel does not define a
> feature string for this extension.  The current effect of this is that
> driver will disable profile feature bit in GCC.  This is OK though
> because we don't, nor do we ever, enable this feature bit, as codegen is
> not affect by the SPE support and more importantly the driver will still
> pass the extension down to the assembler regardless.
>
> Boostrapped aarch64-none-linux-gnu and ran regression tests.
>
> Is it OK for trunk?

I use a similar patch for the last year and half.

Thanks,
Andrew

>
> gcc/ChangeLog:
> 2018-07-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>
>         * config/aarch64/aarch64-option-extensions.def: New entry for profile
>         extension.
>         * config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
>         * doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
>         extension.
>
> gcc/testsuite/ChangeLog:
> 2018-07-09 Andre Vieira <andre.simoesdiasvieira@arm.com>
>
>         * gcc.target/aarch64/profile.c: New test.
James Greenhalgh July 17, 2018, 3:23 p.m. UTC | #3 
On Mon, Jul 09, 2018 at 08:20:53AM -0500, Andre Vieira (lists) wrote:
> Hi,
> 
> This patch adds support for the Statistical Profiling Extension (SPE) on
> AArch64. Even though the compiler will not generate code any differently
> given this extension, it will need to pass it on to the assembler in
> order to let it correctly assemble inline asm containing accesses to the
> extension's system registers.  The same applies when using the
> preprocessor on an assembly file as this first must pass through cc1.
> 
> I left the hwcaps string for SPE empty as the kernel does not define a
> feature string for this extension.  The current effect of this is that
> driver will disable profile feature bit in GCC.  This is OK though
> because we don't, nor do we ever, enable this feature bit, as codegen is
> not affect by the SPE support and more importantly the driver will still
> pass the extension down to the assembler regardless.

Please make these conditions clear in the documentation. Something like.

> +@item profile
> +Enable the Statistical Profiling extension.  This option only changes
> +the behavior of the assembler, and does not change code generation.

Maybe worded better...

> 
> Boostrapped aarch64-none-linux-gnu and ran regression tests.
> 
> Is it OK for trunk?
> 
> gcc/ChangeLog:
> 2018-07-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> 
> 	* config/aarch64/aarch64-option-extensions.def: New entry for profile
> 	extension.
> 	* config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
> 	* doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
> 	extension.
> 
> gcc/testsuite/ChangeLog:
> 2018-07-09 Andre Vieira <andre.simoesdiasvieira@arm.com>
> 
> 	* gcc.target/aarch64/profile.c: New test.

This test will fail for targets with old assemblers. That isn't ideal, we
don't normally add these assembler tests for new instructions for that
reason. Personally I'd drop the test down to a compile-only and scan the
assembler for "+profile".

OK with those changes.

Thanks,
James


> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
> index 5fe5e3f7dddf622a48a5b9458ef30449a886f395..69ab796a4e1a959b89ebb55b599919c442cfb088 100644
> --- a/gcc/config/aarch64/aarch64-option-extensions.def
> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
> @@ -105,4 +105,7 @@ AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML, AARCH64_FL_FP | AARCH64_FL_F
>     Disabling "sve" just disables "sve".  */
>  AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16, 0, "sve")
>  
> +/* Enabling/Disabling "profile" does not enable/disable any other feature.  */
> +AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, "")
> +
>  #undef AARCH64_OPT_EXTENSION
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index f284e74bfb8c9bab2aa22cc6c5a67750cbbba3c2..c1218503bab19323eee1cca8b7e4bea8fbfcf573 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -158,6 +158,9 @@ extern unsigned aarch64_architecture_version;
>  #define AARCH64_FL_SHA3	      (1 << 18)  /* Has ARMv8.4-a SHA3 and SHA512.  */
>  #define AARCH64_FL_F16FML     (1 << 19)  /* Has ARMv8.4-a FP16 extensions.  */
>  
> +/* Statistical Profiling extensions.  */
> +#define AARCH64_FL_PROFILE    (1 << 20)
> +
>  /* Has FP and SIMD.  */
>  #define AARCH64_FL_FPSIMD     (AARCH64_FL_FP | AARCH64_FL_SIMD)
>  
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 56cd122b0d7b420e2b16ceb02907860879d3b9d7..4ca68a563297482afc75abed4a31c106af38caf7 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14813,6 +14813,8 @@ instructions. Use of this option with architectures prior to Armv8.2-A is not su
>  @item sm4
>  Enable the sm3 and sm4 crypto extension.  This also enables Advanced SIMD instructions.
>  Use of this option with architectures prior to Armv8.2-A is not supported.
> +@item profile
> +Enable the Statistical Profiling extension.
>  
>  @end table
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/profile.c b/gcc/testsuite/gcc.target/aarch64/profile.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..db51b4746dd60009d784bc0b37ea99b2f120d856
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/profile.c
> @@ -0,0 +1,9 @@
> +/* { dg-do assemble } */
> +/* { dg-options "-std=gnu99 -march=armv8.2-a+profile" } */
> +
> +int foo (void)
> +{
> +  int ret;
> +  asm ("mrs  %0, pmblimitr_el1" : "=r" (ret));
> +  return ret;
> +}
Andre Vieira (lists) July 19, 2018, 3:47 p.m. UTC | #4 
On 17/07/18 16:23, James Greenhalgh wrote:
> On Mon, Jul 09, 2018 at 08:20:53AM -0500, Andre Vieira (lists) wrote:
>> Hi,
>>
>> This patch adds support for the Statistical Profiling Extension (SPE) on
>> AArch64. Even though the compiler will not generate code any differently
>> given this extension, it will need to pass it on to the assembler in
>> order to let it correctly assemble inline asm containing accesses to the
>> extension's system registers.  The same applies when using the
>> preprocessor on an assembly file as this first must pass through cc1.
>>
>> I left the hwcaps string for SPE empty as the kernel does not define a
>> feature string for this extension.  The current effect of this is that
>> driver will disable profile feature bit in GCC.  This is OK though
>> because we don't, nor do we ever, enable this feature bit, as codegen is
>> not affect by the SPE support and more importantly the driver will still
>> pass the extension down to the assembler regardless.
> 
> Please make these conditions clear in the documentation. Something like.
> 
>> +@item profile
>> +Enable the Statistical Profiling extension.  This option only changes
>> +the behavior of the assembler, and does not change code generation.
> 
> Maybe worded better...
> 
>>
>> Boostrapped aarch64-none-linux-gnu and ran regression tests.
>>
>> Is it OK for trunk?
>>
>> gcc/ChangeLog:
>> 2018-07-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>>
>> 	* config/aarch64/aarch64-option-extensions.def: New entry for profile
>> 	extension.
>> 	* config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
>> 	* doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
>> 	extension.
>>
>> gcc/testsuite/ChangeLog:
>> 2018-07-09 Andre Vieira <andre.simoesdiasvieira@arm.com>
>>
>> 	* gcc.target/aarch64/profile.c: New test.
> 
> This test will fail for targets with old assemblers. That isn't ideal, we
> don't normally add these assembler tests for new instructions for that
> reason. Personally I'd drop the test down to a compile-only and scan the
> assembler for "+profile".
> 
> OK with those changes.
> 

Committed with changes in r262882.

> Thanks,
> James
> 
> 
>> diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
>> index 5fe5e3f7dddf622a48a5b9458ef30449a886f395..69ab796a4e1a959b89ebb55b599919c442cfb088 100644
>> --- a/gcc/config/aarch64/aarch64-option-extensions.def
>> +++ b/gcc/config/aarch64/aarch64-option-extensions.def
>> @@ -105,4 +105,7 @@ AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML, AARCH64_FL_FP | AARCH64_FL_F
>>     Disabling "sve" just disables "sve".  */
>>  AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16, 0, "sve")
>>  
>> +/* Enabling/Disabling "profile" does not enable/disable any other feature.  */
>> +AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, "")
>> +
>>  #undef AARCH64_OPT_EXTENSION
>> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
>> index f284e74bfb8c9bab2aa22cc6c5a67750cbbba3c2..c1218503bab19323eee1cca8b7e4bea8fbfcf573 100644
>> --- a/gcc/config/aarch64/aarch64.h
>> +++ b/gcc/config/aarch64/aarch64.h
>> @@ -158,6 +158,9 @@ extern unsigned aarch64_architecture_version;
>>  #define AARCH64_FL_SHA3	      (1 << 18)  /* Has ARMv8.4-a SHA3 and SHA512.  */
>>  #define AARCH64_FL_F16FML     (1 << 19)  /* Has ARMv8.4-a FP16 extensions.  */
>>  
>> +/* Statistical Profiling extensions.  */
>> +#define AARCH64_FL_PROFILE    (1 << 20)
>> +
>>  /* Has FP and SIMD.  */
>>  #define AARCH64_FL_FPSIMD     (AARCH64_FL_FP | AARCH64_FL_SIMD)
>>  
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 56cd122b0d7b420e2b16ceb02907860879d3b9d7..4ca68a563297482afc75abed4a31c106af38caf7 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -14813,6 +14813,8 @@ instructions. Use of this option with architectures prior to Armv8.2-A is not su
>>  @item sm4
>>  Enable the sm3 and sm4 crypto extension.  This also enables Advanced SIMD instructions.
>>  Use of this option with architectures prior to Armv8.2-A is not supported.
>> +@item profile
>> +Enable the Statistical Profiling extension.
>>  
>>  @end table
>>  
>> diff --git a/gcc/testsuite/gcc.target/aarch64/profile.c b/gcc/testsuite/gcc.target/aarch64/profile.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..db51b4746dd60009d784bc0b37ea99b2f120d856
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/profile.c
>> @@ -0,0 +1,9 @@
>> +/* { dg-do assemble } */
>> +/* { dg-options "-std=gnu99 -march=armv8.2-a+profile" } */
>> +
>> +int foo (void)
>> +{
>> +  int ret;
>> +  asm ("mrs  %0, pmblimitr_el1" : "=r" (ret));
>> +  return ret;
>> +}
>
From bc351f3940bdd26dfe91dd8fba2a00667705aa20 Mon Sep 17 00:00:00 2001
From: Andre Simoes Dias Vieira <andsim01@hw-a20-9.euhpc2.arm.com>
Date: Thu, 5 Jul 2018 10:27:37 +0100
Subject: [PATCH] [PATCH, GCC, AARCH64] Add support for +profile extension

This patch adds support for the Statistical Profiling extensions on AArch64.
Even though the compiler will not generate code any differently given this
extension, it will need to pass it on to the assembler in order to let it
correctly assemble inline asm containing accesses to the extension's system
registers.  The same applie when using the preprocessor on an assembly file as
t his first must pass through cc1.

gcc/ChangeLog:
2018-07-xx  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	* config/aarch64/aarch64-option-extensions.def: New entry for profile
	extension.
	* config/aarch64/aarch64.h (AARCH64_FL_PROFILE): New.
	* doc/invoke.texi (aarch64-feature-modifiers): New entry for profile
	extension.
---
 gcc/config/aarch64/aarch64-option-extensions.def | 3 +++
 gcc/config/aarch64/aarch64.h                     | 3 +++
 gcc/doc/invoke.texi                              | 3 +++
 gcc/testsuite/gcc.target/aarch64/profile.c       | 3 +++
 4 files changed, 12 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/profile.c

diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
index 5fe5e3f7dddf622a48a5b9458ef30449a886f395..69ab796a4e1a959b89ebb55b599919c442cfb088 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -105,4 +105,7 @@ AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML, AARCH64_FL_FP | AARCH64_FL_F
    Disabling "sve" just disables "sve".  */
 AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16, 0, "sve")
 
+/* Enabling/Disabling "profile" does not enable/disable any other feature.  */
+AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, "")
+
 #undef AARCH64_OPT_EXTENSION
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index f284e74bfb8c9bab2aa22cc6c5a67750cbbba3c2..c1218503bab19323eee1cca8b7e4bea8fbfcf573 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -158,6 +158,9 @@ extern unsigned aarch64_architecture_version;
 #define AARCH64_FL_SHA3	      (1 << 18)  /* Has ARMv8.4-a SHA3 and SHA512.  */
 #define AARCH64_FL_F16FML     (1 << 19)  /* Has ARMv8.4-a FP16 extensions.  */
 
+/* Statistical Profiling extensions.  */
+#define AARCH64_FL_PROFILE    (1 << 20)
+
 /* Has FP and SIMD.  */
 #define AARCH64_FL_FPSIMD     (AARCH64_FL_FP | AARCH64_FL_SIMD)
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 56cd122b0d7b420e2b16ceb02907860879d3b9d7..2a5e059a2c1c43eb56de9504d88b85b9e8de1bfc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14813,6 +14813,9 @@ instructions. Use of this option with architectures prior to Armv8.2-A is not su
 @item sm4
 Enable the sm3 and sm4 crypto extension.  This also enables Advanced SIMD instructions.
 Use of this option with architectures prior to Armv8.2-A is not supported.
+@item profile
+Enable the Statistical Profiling extension.  This option is only to enable the
+extension at the assembler level and does not affect code generation.
 
 @end table
 
diff --git a/gcc/testsuite/gcc.target/aarch64/profile.c b/gcc/testsuite/gcc.target/aarch64/profile.c
new file mode 100644
index 0000000000000000000000000000000000000000..c2dd1124ca6ef1c71e7680899002d1c06d7c8b4a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/profile.c
@@ -0,0 +1,3 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv8.2-a+profile" } */
+/* { dg-final { scan-assembler "\\.arch armv8.2-a\[\^\n\]*\\+profile\[\^\n\]*\n" } } */
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
index 5fe5e3f7dddf622a48a5b9458ef30449a886f395..69ab796a4e1a959b89ebb55b599919c442cfb088 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -105,4 +105,7 @@  AARCH64_OPT_EXTENSION("fp16fml", AARCH64_FL_F16FML, AARCH64_FL_FP | AARCH64_FL_F
    Disabling "sve" just disables "sve".  */
 AARCH64_OPT_EXTENSION("sve", AARCH64_FL_SVE, AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16, 0, "sve")
 
+/* Enabling/Disabling "profile" does not enable/disable any other feature.  */
+AARCH64_OPT_EXTENSION("profile", AARCH64_FL_PROFILE, 0, 0, "")
+
 #undef AARCH64_OPT_EXTENSION
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index f284e74bfb8c9bab2aa22cc6c5a67750cbbba3c2..c1218503bab19323eee1cca8b7e4bea8fbfcf573 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -158,6 +158,9 @@  extern unsigned aarch64_architecture_version;
 #define AARCH64_FL_SHA3	      (1 << 18)  /* Has ARMv8.4-a SHA3 and SHA512.  */
 #define AARCH64_FL_F16FML     (1 << 19)  /* Has ARMv8.4-a FP16 extensions.  */
 
+/* Statistical Profiling extensions.  */
+#define AARCH64_FL_PROFILE    (1 << 20)
+
 /* Has FP and SIMD.  */
 #define AARCH64_FL_FPSIMD     (AARCH64_FL_FP | AARCH64_FL_SIMD)
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 56cd122b0d7b420e2b16ceb02907860879d3b9d7..4ca68a563297482afc75abed4a31c106af38caf7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14813,6 +14813,8 @@  instructions. Use of this option with architectures prior to Armv8.2-A is not su
 @item sm4
 Enable the sm3 and sm4 crypto extension.  This also enables Advanced SIMD instructions.
 Use of this option with architectures prior to Armv8.2-A is not supported.
+@item profile
+Enable the Statistical Profiling extension.
 
 @end table
 
diff --git a/gcc/testsuite/gcc.target/aarch64/profile.c b/gcc/testsuite/gcc.target/aarch64/profile.c
new file mode 100644
index 0000000000000000000000000000000000000000..db51b4746dd60009d784bc0b37ea99b2f120d856
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/profile.c
@@ -0,0 +1,9 @@ 
+/* { dg-do assemble } */
+/* { dg-options "-std=gnu99 -march=armv8.2-a+profile" } */
+
+int foo (void)
+{
+  int ret;
+  asm ("mrs  %0, pmblimitr_el1" : "=r" (ret));
+  return ret;
+}