diff mbox series

[STAGE,4] aarch64: Disable sysreg feature gating

Message ID f5c69138-23ea-c616-ffef-746669f0ab87@e124511.cambridge.arm.com
State New
Headers
Series [STAGE,4] aarch64: Disable sysreg feature gating |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm fail Patch failed to apply

Commit Message

Alice Carlotti April 15, 2025, 6:21 p.m. UTC 
  This applies to the sysreg read/write intrinsics __arm_[wr]sr*.  It does
not depend on changes to Binutils, because GCC converts recognised
sysreg names to an encoding based form, which is already ungated in Binutils.

We have, however, agreed to make an equivalent change in Binutils (which
would then disable feature gating for sysreg accesses in inline
assembly), but this has not yet been posted upstream.

In the future we may introduce a new flag to renable some checking,
but these checks could not be comprehensive because many system
registers depend on architecture features that don't have corresponding
GCC/GAS --march options.  This would also depend on addressing numerous
inconsistencies in the existing list of sysreg feature dependencies.

---

Ok for master now? And how about backporting to gcc 14? I do recognise that
this is late in stage 4, sorry - it slipped through the gaps of being
Binutils-adjacent work with a different deadline.

Thanks,
Alice



gcc/ChangeLog:

	* config/aarch64/aarch64.cc
	(aarch64_valid_sysreg_name_p): Remove feature check.
	(aarch64_retrieve_sysreg): Ditto.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/acle/rwsr-ungated.c: New test.

Comments

Richard Sandiford April 15, 2025, 6:39 p.m. UTC | #1 
Alice Carlotti <alice.carlotti@arm.com> writes:
> This applies to the sysreg read/write intrinsics __arm_[wr]sr*.  It does
> not depend on changes to Binutils, because GCC converts recognised
> sysreg names to an encoding based form, which is already ungated in Binutils.
>
> We have, however, agreed to make an equivalent change in Binutils (which
> would then disable feature gating for sysreg accesses in inline
> assembly), but this has not yet been posted upstream.
>
> In the future we may introduce a new flag to renable some checking,
> but these checks could not be comprehensive because many system
> registers depend on architecture features that don't have corresponding
> GCC/GAS --march options.  This would also depend on addressing numerous
> inconsistencies in the existing list of sysreg feature dependencies.
>
> ---
>
> Ok for master now? And how about backporting to gcc 14? I do recognise that
> this is late in stage 4, sorry - it slipped through the gaps of being
> Binutils-adjacent work with a different deadline.

OK for trunk and backports.  I'm disappointed that I didn't notice
the lack of dg-error tests for the code being removed.

Thanks,
Richard

>
> Thanks,
> Alice
>
>
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64.cc
> 	(aarch64_valid_sysreg_name_p): Remove feature check.
> 	(aarch64_retrieve_sysreg): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/acle/rwsr-ungated.c: New test.
>
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 4e801146c60a52c7ef6f8c0f92b1b922e729c234..433ec975d7e4e9d7130fe49eac37f4ebfb880416 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -31073,8 +31073,6 @@ aarch64_valid_sysreg_name_p (const char *regname)
>    const sysreg_t *sysreg = aarch64_lookup_sysreg_map (regname);
>    if (sysreg == NULL)
>      return aarch64_is_implem_def_reg (regname);
> -  if (sysreg->arch_reqs)
> -    return bool (aarch64_isa_flags & sysreg->arch_reqs);
>    return true;
>  }
>  
> @@ -31098,8 +31096,6 @@ aarch64_retrieve_sysreg (const char *regname, bool write_p, bool is128op)
>    if ((write_p && (sysreg->properties & F_REG_READ))
>        || (!write_p && (sysreg->properties & F_REG_WRITE)))
>      return NULL;
> -  if ((~aarch64_isa_flags & sysreg->arch_reqs) != 0)
> -    return NULL;
>    return sysreg->encoding;
>  }
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-ungated.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-ungated.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d67a42673733cdb128fd62d465fa122037ae531d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-ungated.c
> @@ -0,0 +1,13 @@
> +/* Test that __arm_[r,w]sr intrinsics aren't gated (by default).  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-march=armv8-a" } */
> +
> +#include <arm_acle.h>
> +
> +uint64_t
> +foo (uint64_t a)
> +{
> +  __arm_wsr64 ("zcr_el1", a);
> +  return __arm_rsr64 ("smcr_el1");
> +}
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 4e801146c60a52c7ef6f8c0f92b1b922e729c234..433ec975d7e4e9d7130fe49eac37f4ebfb880416 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -31073,8 +31073,6 @@  aarch64_valid_sysreg_name_p (const char *regname)
   const sysreg_t *sysreg = aarch64_lookup_sysreg_map (regname);
   if (sysreg == NULL)
     return aarch64_is_implem_def_reg (regname);
-  if (sysreg->arch_reqs)
-    return bool (aarch64_isa_flags & sysreg->arch_reqs);
   return true;
 }
 
@@ -31098,8 +31096,6 @@  aarch64_retrieve_sysreg (const char *regname, bool write_p, bool is128op)
   if ((write_p && (sysreg->properties & F_REG_READ))
       || (!write_p && (sysreg->properties & F_REG_WRITE)))
     return NULL;
-  if ((~aarch64_isa_flags & sysreg->arch_reqs) != 0)
-    return NULL;
   return sysreg->encoding;
 }
 
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-ungated.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-ungated.c
new file mode 100644
index 0000000000000000000000000000000000000000..d67a42673733cdb128fd62d465fa122037ae531d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-ungated.c
@@ -0,0 +1,13 @@ 
+/* Test that __arm_[r,w]sr intrinsics aren't gated (by default).  */
+
+/* { dg-do compile } */
+/* { dg-options "-march=armv8-a" } */
+
+#include <arm_acle.h>
+
+uint64_t
+foo (uint64_t a)
+{
+  __arm_wsr64 ("zcr_el1", a);
+  return __arm_rsr64 ("smcr_el1");
+}