On Fri, Sep 19, 2025 at 3:42 AM Srinath Parvathaneni
<[email protected]> wrote:
>
> Hi,
>
> >On Thu, Sep 18, 2025 at 10:23 AM Srinath Parvathaneni
> ><[email protected]> wrote:
> >>
> >> v1 -> v2 changes:
> >>
> >> * Fixed the wrong conditional check.
> >> * Fixed the typos in the testcases.
> >> * Added support for inline assembly checking.
> >> * Added new tests for inline assembly sysreg checking.
> >> -------------------
> >> 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.
> >>
> >> For inline assembly, sysreg checks are not performed by CC1 and are
> >> instead delegated to the assembler. By default, the assembler does not
> >> perform these checks either. With this patch, the -menable-sysreg-checking
> >> flag passed to the compiler will also be propagated to the assembler,
> >> enabling sysreg checking for inline assembly.
> >>
> >> Regression tested on aarch64-none-elf and found no regressions.
> >>
> >> Ok for trunk?
> >>
> >> Regards,
> >> Srinath.
> >>
> >> 2025-09-18 Srinath Parvathaneni <[email protected]>
> >>
> >> gcc/ChangeLog:
> >>
> >> * config/aarch64/aarch64-elf.h (ASM_SPEC): Update the macro.
> >> * config/aarch64/aarch64.cc (aarch64_valid_sysreg_name_p):
> >> Add feature check condition.
> >> (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/asm-inlined-sysreg-1.c: New test.
> >> * gcc.target/aarch64/acle/asm-inlined-sysreg-2.c: Likewise.
> >> * gcc.target/aarch64/acle/rwsr-gated-1.c: Likewise.
> >> * gcc.target/aarch64/acle/rwsr-gated-2.c: Likewise.
> >> ---
> >> gcc/config/aarch64/aarch64-elf.h | 1 +
> >> gcc/config/aarch64/aarch64.cc | 6 ++++
> >> gcc/config/aarch64/aarch64.opt | 5 ++++
> >> gcc/doc/invoke.texi | 6 ++++
> >> .../aarch64/acle/asm-inlined-sysreg-1.c | 27 ++++++++++++++++++
> >> .../aarch64/acle/asm-inlined-sysreg-2.c | 28 +++++++++++++++++++
> >> .../gcc.target/aarch64/acle/rwsr-gated-1.c | 14 ++++++++++
> >> .../gcc.target/aarch64/acle/rwsr-gated-2.c | 14 ++++++++++
> >> 8 files changed, 101 insertions(+)
> >> create mode 100644
> >> gcc/testsuite/gcc.target/aarch64/acle/asm-inlined-sysreg-1.c
> >> create mode 100644
> >> gcc/testsuite/gcc.target/aarch64/acle/asm-inlined-sysreg-2.c
> >> 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-elf.h
> >> b/gcc/config/aarch64/aarch64-elf.h
> >> index f6ebb723715..57c5a319d7c 100644
> >> --- a/gcc/config/aarch64/aarch64-elf.h
> >> +++ b/gcc/config/aarch64/aarch64-elf.h
> >> @@ -136,6 +136,7 @@
> >> #define ASM_SPEC "\
> >> %{mbig-endian:-EB} \
> >> %{mlittle-endian:-EL} \
> >> +%{menable-sysreg-checking} \
> >> %(asm_cpu_spec)" \
> >> ASM_MABI_SPEC
> >> #endif
> >> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> >> index ef9c16598c0..7897c2575c1 100644
> >> --- a/gcc/config/aarch64/aarch64.cc
> >> +++ b/gcc/config/aarch64/aarch64.cc
> >> @@ -31702,6 +31702,9 @@ 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
> >> + && ((~aarch64_isa_flags & sysreg->arch_reqs) != 0))
> >> + return bool (aarch64_isa_flags & sysreg->arch_reqs);
> >
> >I think it is better to write it like:
> >return (aarch64_isa_flags & sysreg->arch_reqs) != 0;
> >
>
> Sure, I will make this change.
>
> >> return true;
> >> }
> >>
> >> @@ -31725,6 +31728,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 76ecea550f3..4874207d7eb 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/asm-inlined-sysreg-1.c
> >> b/gcc/testsuite/gcc.target/aarch64/acle/asm-inlined-sysreg-1.c
> >> new file mode 100644
> >> index 00000000000..9eecf3b053e
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/aarch64/acle/asm-inlined-sysreg-1.c
> >> @@ -0,0 +1,27 @@
> >> +/* { dg-do assemble } */
> >> +/* { dg-additional-options "-march=armv8-a -menable-sysreg-checking -###"
> >> } */
> >
> >Since you did the option for the spec in aarch64-elf.h. This testcase
> >should only run for elf targets.
> >So something like:
> >/* { dg-do assemble { target elf } } */
> >
> >The other testcases that use `dg-do assemble` might need that too but
> >I have not checked 100% on that.
> >When aarch64-mingw support for the spec is added for this option, then
> >the testcases can be changed/fixed to run there.
> The changes in this patch apply to both ELF and Linux targets. Although I
> only updated ASM_SPEC in aarch64-elf.h, those updates are also pulled
> in by the aarch64*-*-linux* target (see the snippet from config.gcc below),
> so there was no need to modify aarch64-linux.h. You are correct, however,
> that they do not cover other targets such as aarch64-mingw.
> From config.gcc
> aarch64*-*-linux*)
> tm_file="${tm_file} elfos.h gnu-user.h linux.h glibc-stdint.h"
> tm_file="${tm_file} aarch64/aarch64-elf.h aarch64/aarch64-errata.h
> aarch64/aarch64-linux.h"
>
> I will make the tests in this patch run only for ELF and Linux targets using
> following check:
>
> /* { dg-do assemble { target aarch64*-*-elf* aarch64*-*-linux* } } */
I was suggesting to use check_effective_target_elf instead of the above.
So it would just be:
/* { dg-do assemble { target elf } } */
And then inside check_effective_target_aarch64_sysreg_guarding_ok instead of:
if { [istarget aarch64*-*-elf] || [istarget aarch64*-*-linux*] } {
Do:
if { [istarget aarch64*-*-* ] && [check_effective_target_elf] } {
This will allow you to catch fuchsia, rtems, netbsd, gnu (hurd),
vxworks freebsd (which might have issues if gas is not as), etc.
Thanks,
Andrew
>
> Regards,
> Srinath.
>
> >
> >Thanks,
> >Andrew Pinski
