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* } } */
Regards,
Srinath.
>
>Thanks,
>Andrew Pinski
