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.parvathan...@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.parvathan...@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
> >

Reply via email to