diff mbox series

[v1] aarch64: Add support for menable-sysreg-checking flag.

Message ID 20250912115505.2831286-1-srinath.parvathaneni@arm.com
State Superseded
Headers
Series [v1] aarch64: Add support for menable-sysreg-checking flag. |

Commit Message

Srinath Parvathaneni Sept. 12, 2025, 11:55 a.m. UTC 
  Hi All,

In the current Binutils we have disabled the feature gating for sysreg
by default and we have introduced a new flag "-meanble-sysreg-checking"
to renable some of this checking.

However in GCC, we have disabled the feature gating of sysreg to read/write
intrinsics __arm_[wr]sr* and we have not added any mechanism to check the
feature gating if needed similar to Binutils.

This patch adds the support for the flag "-meanble-sysreg-checking" which
renables some of the feature checking of sysreg to read/write intrinsics
__arm_[wr]sr* similar to Binutils.

Regression tested on aarch64-none-elf and found no regressions.

Ok for trunk?

Regards,
Srinath.

2025-09-12  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

gcc/ChangeLog:

        * config/aarch64/aarch64.cc (aarch64_valid_sysreg_name_p): Add feature
	check.
        (aarch64_retrieve_sysreg): Likewise.
        * config/aarch64/aarch64.opt (menable-sysreg-checking): Define new flag.
        * doc/invoke.texi (menable-sysreg-checking): Document new flag.

gcc/testsuite/ChangeLog:

        * gcc.target/aarch64/acle/rwsr-gated-1.c: New test.
        * gcc.target/aarch64/acle/rwsr-gated-2.c: Likewise.
---
 gcc/config/aarch64/aarch64.cc                      |  5 +++++
 gcc/config/aarch64/aarch64.opt                     |  5 +++++
 gcc/doc/invoke.texi                                |  6 ++++++
 .../gcc.target/aarch64/acle/rwsr-gated-1.c         | 13 +++++++++++++
 .../gcc.target/aarch64/acle/rwsr-gated-2.c         | 14 ++++++++++++++
 5 files changed, 43 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c

Comments

Andrew Pinski Sept. 12, 2025, 3:30 p.m. UTC | #1 
On Fri, Sep 12, 2025 at 4:58 AM Srinath Parvathaneni
<srinath.parvathaneni@arm.com> wrote:
>
> Hi All,
>
> In the current Binutils we have disabled the feature gating for sysreg
> by default and we have introduced a new flag "-meanble-sysreg-checking"
> to renable some of this checking.
>
> However in GCC, we have disabled the feature gating of sysreg to read/write
> intrinsics __arm_[wr]sr* and we have not added any mechanism to check the
> feature gating if needed similar to Binutils.
>
> This patch adds the support for the flag "-meanble-sysreg-checking" which
> renables some of the feature checking of sysreg to read/write intrinsics
> __arm_[wr]sr* similar to Binutils.
>
> Regression tested on aarch64-none-elf and found no regressions.
>
> Ok for trunk?

LGTM.

Thanks,
Andrew

>
> Regards,
> Srinath.
>
> 2025-09-12  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
>
> gcc/ChangeLog:
>
>         * config/aarch64/aarch64.cc (aarch64_valid_sysreg_name_p): Add feature
>         check.
>         (aarch64_retrieve_sysreg): Likewise.
>         * config/aarch64/aarch64.opt (menable-sysreg-checking): Define new flag.
>         * doc/invoke.texi (menable-sysreg-checking): Document new flag.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/aarch64/acle/rwsr-gated-1.c: New test.
>         * gcc.target/aarch64/acle/rwsr-gated-2.c: Likewise.
> ---
>  gcc/config/aarch64/aarch64.cc                      |  5 +++++
>  gcc/config/aarch64/aarch64.opt                     |  5 +++++
>  gcc/doc/invoke.texi                                |  6 ++++++
>  .../gcc.target/aarch64/acle/rwsr-gated-1.c         | 13 +++++++++++++
>  .../gcc.target/aarch64/acle/rwsr-gated-2.c         | 14 ++++++++++++++
>  5 files changed, 43 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index ef9c16598c0..5eb7e4dc17c 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -31702,6 +31702,8 @@ 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 (aarch64_enable_sysreg_guarding && sysreg->arch_reqs)
> +    return bool (aarch64_isa_flags & sysreg->arch_reqs);
>    return true;
>  }
>
> @@ -31725,6 +31727,9 @@ 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_enable_sysreg_guarding
> +      && ((~aarch64_isa_flags & sysreg->arch_reqs) != 0))
> +    return NULL;
>    return sysreg->encoding;
>  }
>
> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> index 9ca753e6a88..5df5a159459 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -82,6 +82,11 @@ mbig-endian
>  Target RejectNegative Mask(BIG_END)
>  Assume target CPU is configured as big endian.
>
> +menable-sysreg-checking
> +Target RejectNegative Var(aarch64_enable_sysreg_guarding) Init(0)
> +Generates an error message if an attempt is made to access a system register
> +which will not execute on the target architecture.
> +
>  mgeneral-regs-only
>  Target RejectNegative Mask(GENERAL_REGS_ONLY) Save
>  Generate code which uses only the general registers.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index d0c13d4a24e..f2a4929f793 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -823,6 +823,7 @@ Objective-C and Objective-C++ Dialects}.
>
>  @emph{AArch64 Options} (@ref{AArch64 Options})
>  @gccoptlist{-mabi=@var{name}  -mbig-endian  -mlittle-endian
> +-menable-sysreg-checking
>  -mgeneral-regs-only
>  -mcmodel=tiny  -mcmodel=small  -mcmodel=large
>  -mstrict-align  -mno-strict-align
> @@ -22091,6 +22092,11 @@ The @samp{ilp32} model is deprecated.
>  Generate big-endian code.  This is the default when GCC is configured for an
>  @samp{aarch64_be-*-*} target.
>
> +@opindex menable-sysreg-checking
> +@item -menable-sysreg-checking
> +Generates an error message if an attempt is made to access a system register
> +which will not execute on the target architecture.
> +
>  @opindex mgeneral-regs-only
>  @item -mgeneral-regs-only
>  Generate code which uses only the general-purpose registers.  This will prevent
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
> new file mode 100644
> index 00000000000..b9fb39cf0c6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
> @@ -0,0 +1,13 @@
> +/* Ensure that system register are properly gated on the feature flags, when the
> +   guarding is enabled through "-menable-sysreg-checking" command line flag.  */
> +/* { dg-do compile } */
> +/* { dg-options "-menable-sysreg-checking -march=armv8-a+sve2+sme" } */
> +
> +#include <arm_acle.h>
> +
> +uint64_t
> +foo (uint64_t a)
> +{
> +  __arm_wsr64 ("zcr_el1", a); /* { { dg-final { scan-assembler "msr\ts3_0_c1_c2_0, x0" } } */
> +  return __arm_rsr64 ("smcr_el1"); /* { { dg-final { scan-assembler "mrs\tx0, s3_0_c1_c2_6" } } */
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
> new file mode 100644
> index 00000000000..ef143af3ec8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-menable-sysreg-checking -march=armv8-a" } */
> +/* Ensure the system register are rejected by compiler when guarding is
> +   enabled through "-menable-sysreg-checking" command line flag and proper
> +   feature flags are not passed.  */
> +
> +#include <arm_acle.h>
> +
> +uint64_t
> +foo (uint64_t a)
> +{
> +  __arm_wsr64 ("zcr_el1", a); /* { dg-error "invalid system register name 'zcr_el1'" } */
> +  return __arm_rsr64 ("smcr_el1"); /* { dg-error "invalid system register name 'smcr_el1'" } */
> +}
> --
> 2.25.1
>
Alice Carlotti Sept. 15, 2025, 12:23 a.m. UTC | #2 
On Fri, Sep 12, 2025 at 08:30:55AM -0700, Andrew Pinski wrote:
> On Fri, Sep 12, 2025 at 4:58 AM Srinath Parvathaneni
> <srinath.parvathaneni@arm.com> wrote:
> >
> > Hi All,
> >
> > In the current Binutils we have disabled the feature gating for sysreg
> > by default and we have introduced a new flag "-meanble-sysreg-checking"
> > to renable some of this checking.
> >
> > However in GCC, we have disabled the feature gating of sysreg to read/write
> > intrinsics __arm_[wr]sr* and we have not added any mechanism to check the
> > feature gating if needed similar to Binutils.
> >
> > This patch adds the support for the flag "-meanble-sysreg-checking" which
> > renables some of the feature checking of sysreg to read/write intrinsics
> > __arm_[wr]sr* similar to Binutils.
> >
> > Regression tested on aarch64-none-elf and found no regressions.
> >
> > Ok for trunk?
> 
> LGTM.
> 
> Thanks,
> Andrew

Some issues noted below.

It might be nice if we could pass this flag through to the assembler, to enable
checks for inline assembly as well, but I'm not sure how complicated that would
be to support.

> 
> >
> > Regards,
> > Srinath.
> >
> > 2025-09-12  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
> >
> > gcc/ChangeLog:
> >
> >         * config/aarch64/aarch64.cc (aarch64_valid_sysreg_name_p): Add feature
> >         check.
> >         (aarch64_retrieve_sysreg): Likewise.
> >         * config/aarch64/aarch64.opt (menable-sysreg-checking): Define new flag.
> >         * doc/invoke.texi (menable-sysreg-checking): Document new flag.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/aarch64/acle/rwsr-gated-1.c: New test.
> >         * gcc.target/aarch64/acle/rwsr-gated-2.c: Likewise.
> > ---
> >  gcc/config/aarch64/aarch64.cc                      |  5 +++++
> >  gcc/config/aarch64/aarch64.opt                     |  5 +++++
> >  gcc/doc/invoke.texi                                |  6 ++++++
> >  .../gcc.target/aarch64/acle/rwsr-gated-1.c         | 13 +++++++++++++
> >  .../gcc.target/aarch64/acle/rwsr-gated-2.c         | 14 ++++++++++++++
> >  5 files changed, 43 insertions(+)
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
> >
> > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> > index ef9c16598c0..5eb7e4dc17c 100644
> > --- a/gcc/config/aarch64/aarch64.cc
> > +++ b/gcc/config/aarch64/aarch64.cc
> > @@ -31702,6 +31702,8 @@ 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 (aarch64_enable_sysreg_guarding && sysreg->arch_reqs)
> > +    return bool (aarch64_isa_flags & sysreg->arch_reqs);
> >    return true;
> >  }

This condition is wrong - it should match the condition in the hunk below.  (It
was wrong before I removed it as well, but I only noted this in the email and
not in the commit message).

> >
> > @@ -31725,6 +31727,9 @@ 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_enable_sysreg_guarding
> > +      && ((~aarch64_isa_flags & sysreg->arch_reqs) != 0))
> > +    return NULL;
> >    return sysreg->encoding;
> >  }
> >
> > diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> > index 9ca753e6a88..5df5a159459 100644
> > --- a/gcc/config/aarch64/aarch64.opt
> > +++ b/gcc/config/aarch64/aarch64.opt
> > @@ -82,6 +82,11 @@ mbig-endian
> >  Target RejectNegative Mask(BIG_END)
> >  Assume target CPU is configured as big endian.
> >
> > +menable-sysreg-checking
> > +Target RejectNegative Var(aarch64_enable_sysreg_guarding) Init(0)
> > +Generates an error message if an attempt is made to access a system register
> > +which will not execute on the target architecture.
> > +
> >  mgeneral-regs-only
> >  Target RejectNegative Mask(GENERAL_REGS_ONLY) Save
> >  Generate code which uses only the general registers.
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index d0c13d4a24e..f2a4929f793 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -823,6 +823,7 @@ Objective-C and Objective-C++ Dialects}.
> >
> >  @emph{AArch64 Options} (@ref{AArch64 Options})
> >  @gccoptlist{-mabi=@var{name}  -mbig-endian  -mlittle-endian
> > +-menable-sysreg-checking
> >  -mgeneral-regs-only
> >  -mcmodel=tiny  -mcmodel=small  -mcmodel=large
> >  -mstrict-align  -mno-strict-align
> > @@ -22091,6 +22092,11 @@ The @samp{ilp32} model is deprecated.
> >  Generate big-endian code.  This is the default when GCC is configured for an
> >  @samp{aarch64_be-*-*} target.
> >
> > +@opindex menable-sysreg-checking
> > +@item -menable-sysreg-checking
> > +Generates an error message if an attempt is made to access a system register
> > +which will not execute on the target architecture.
> > +
> >  @opindex mgeneral-regs-only
> >  @item -mgeneral-regs-only
> >  Generate code which uses only the general-purpose registers.  This will prevent
> > diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
> > new file mode 100644
> > index 00000000000..b9fb39cf0c6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
> > @@ -0,0 +1,13 @@
> > +/* Ensure that system register are properly gated on the feature flags, when the
typo: s/register/registers/

> > +   guarding is enabled through "-menable-sysreg-checking" command line flag.  */
> > +/* { dg-do compile } */
> > +/* { dg-options "-menable-sysreg-checking -march=armv8-a+sve2+sme" } */
> > +
> > +#include <arm_acle.h>
> > +
> > +uint64_t
> > +foo (uint64_t a)
> > +{
> > +  __arm_wsr64 ("zcr_el1", a); /* { { dg-final { scan-assembler "msr\ts3_0_c1_c2_0, x0" } } */
> > +  return __arm_rsr64 ("smcr_el1"); /* { { dg-final { scan-assembler "mrs\tx0, s3_0_c1_c2_6" } } */
> > +}
> > diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
> > new file mode 100644
> > index 00000000000..ef143af3ec8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
> > @@ -0,0 +1,14 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-menable-sysreg-checking -march=armv8-a" } */
> > +/* Ensure the system register are rejected by compiler when guarding is
Typo: "Ensure that system registers are..."

> > +   enabled through "-menable-sysreg-checking" command line flag and proper
> > +   feature flags are not passed.  */
> > +
> > +#include <arm_acle.h>
> > +
> > +uint64_t
> > +foo (uint64_t a)
> > +{
> > +  __arm_wsr64 ("zcr_el1", a); /* { dg-error "invalid system register name 'zcr_el1'" } */
> > +  return __arm_rsr64 ("smcr_el1"); /* { dg-error "invalid system register name 'smcr_el1'" } */
> > +}
> > --
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index ef9c16598c0..5eb7e4dc17c 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -31702,6 +31702,8 @@  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 (aarch64_enable_sysreg_guarding && sysreg->arch_reqs)
+    return bool (aarch64_isa_flags & sysreg->arch_reqs);
   return true;
 }
 
@@ -31725,6 +31727,9 @@  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_enable_sysreg_guarding
+      && ((~aarch64_isa_flags & sysreg->arch_reqs) != 0))
+    return NULL;
   return sysreg->encoding;
 }
 
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 9ca753e6a88..5df5a159459 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -82,6 +82,11 @@  mbig-endian
 Target RejectNegative Mask(BIG_END)
 Assume target CPU is configured as big endian.
 
+menable-sysreg-checking
+Target RejectNegative Var(aarch64_enable_sysreg_guarding) Init(0)
+Generates an error message if an attempt is made to access a system register
+which will not execute on the target architecture.
+
 mgeneral-regs-only
 Target RejectNegative Mask(GENERAL_REGS_ONLY) Save
 Generate code which uses only the general registers.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d0c13d4a24e..f2a4929f793 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -823,6 +823,7 @@  Objective-C and Objective-C++ Dialects}.
 
 @emph{AArch64 Options} (@ref{AArch64 Options})
 @gccoptlist{-mabi=@var{name}  -mbig-endian  -mlittle-endian
+-menable-sysreg-checking
 -mgeneral-regs-only
 -mcmodel=tiny  -mcmodel=small  -mcmodel=large
 -mstrict-align  -mno-strict-align
@@ -22091,6 +22092,11 @@  The @samp{ilp32} model is deprecated.
 Generate big-endian code.  This is the default when GCC is configured for an
 @samp{aarch64_be-*-*} target.
 
+@opindex menable-sysreg-checking
+@item -menable-sysreg-checking
+Generates an error message if an attempt is made to access a system register
+which will not execute on the target architecture.
+
 @opindex mgeneral-regs-only
 @item -mgeneral-regs-only
 Generate code which uses only the general-purpose registers.  This will prevent
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
new file mode 100644
index 00000000000..b9fb39cf0c6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-1.c
@@ -0,0 +1,13 @@ 
+/* Ensure that system register are properly gated on the feature flags, when the
+   guarding is enabled through "-menable-sysreg-checking" command line flag.  */
+/* { dg-do compile } */
+/* { dg-options "-menable-sysreg-checking -march=armv8-a+sve2+sme" } */
+
+#include <arm_acle.h>
+
+uint64_t
+foo (uint64_t a)
+{
+  __arm_wsr64 ("zcr_el1", a); /* { { dg-final { scan-assembler "msr\ts3_0_c1_c2_0, x0" } } */
+  return __arm_rsr64 ("smcr_el1"); /* { { dg-final { scan-assembler "mrs\tx0, s3_0_c1_c2_6" } } */
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
new file mode 100644
index 00000000000..ef143af3ec8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-gated-2.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-options "-menable-sysreg-checking -march=armv8-a" } */
+/* Ensure the system register are rejected by compiler when guarding is
+   enabled through "-menable-sysreg-checking" command line flag and proper
+   feature flags are not passed.  */
+
+#include <arm_acle.h>
+
+uint64_t
+foo (uint64_t a)
+{
+  __arm_wsr64 ("zcr_el1", a); /* { dg-error "invalid system register name 'zcr_el1'" } */
+  return __arm_rsr64 ("smcr_el1"); /* { dg-error "invalid system register name 'smcr_el1'" } */
+}