Since this is an RFC, it would probably be more helpful to get review
comments about the design or the adherence to the spec.  I'll need to
look into things a bit more for that, though, so I'm afraid the below
is more implementation trivia.

Matthieu Longo <matthieu.lo...@arm.com> writes:
> [...]
> diff --git a/gcc/config.in b/gcc/config.in
> index 7fcabbe5061..1309ba2b133 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -379,6 +379,12 @@
>  #endif
>  
>  
> +/* Define if your assembler supports GCS build attributes. */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_AS_BUILD_ATTRIBUTES_GCS
> +#endif

How about making this HAVE_AS_BUILD_AEABI_ATTRIBUTE?  Or is the idea
that each feature that uses build attributes would need its own
configure check?

> [...]
> @@ -24697,6 +24724,20 @@ aarch64_start_file (void)
>     asm_fprintf (asm_out_file, "\t.arch %s\n",
>               aarch64_last_printed_arch_string.c_str ());
>  
> +/* Emit gcs build attributes only when building a native AArch64-hosted
> +   compiler.  */
> +#if defined(__aarch64__)

Why is this restricted to native compilers?  We should avoid that
if at all possible.

> +  /* Check the current assembly supports gcs build attributes, if not

"Check whether the current assembler supports..."

> +     fallback to .note.gnu.property section.  */
> +  #ifdef HAVE_AS_BUILD_ATTRIBUTES_GCS

It would be good to add:

#ifndef HAVE_AS_BUILD_ATTRIBUTES_GCS
#define HAVE_AS_BUILD_ATTRIBUTES_GCS 0
#endif

somewhere, so that we can use HAVE_AS_BUILD_ATTRIBUTES_GCS in C++
conditions as well as preprocessor conditions.

> +    if (aarch64_gcs_enabled ())
> +      {
> +     aarch64_emit_aeabi_subsection (".aeabi-feature-and-bits", 1, 0);
> +     aarch64_emit_aeabi_attribute ("Tag_Feature_GCS", 3, 1);
> +      }
> +  #endif
> +#endif
> +
>     default_file_start ();
>  }
> [...]
> diff --git 
> a/gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp 
> b/gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp
> new file mode 100644
> index 00000000000..ea47e209227
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp
> @@ -0,0 +1,46 @@
> +# Copyright (C) 2024 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with GCC; see the file COPYING3.  If not see
> +# <http://www.gnu.org/licenses/>.
> +
> +# GCC testsuite that uses the `dg.exp' driver.
> +
> +# Exit immediately if this isn't an AArch64 target.
> +if ![istarget aarch64*-*-*] then {
> +  return
> +}
> +
> +# Load support procs.
> +load_lib gcc-dg.exp
> +
> +# Initialize `dg'.
> +dg-init
> +
> +proc check_effective_target_gas_has_build_attributes { } {
> +    return [check_no_compiler_messages gas_has_build_attributes object {
> +     /* Assembly */
> +     .set ATTR_TYPE_uleb128,   0
> +     .set ATTR_TYPE_asciz,     1
> +     .set Tag_Feature_GCS,     3
> +     .aeabi_subsection .aeabi-feature-and-bits, 1, ATTR_TYPE_uleb128
> +     .aeabi_attribute Tag_Feature_GCS, 1
> +    }]
> +}

This should go in lib/target-supports.exp instead, probably as
"aarch64_gas_has...".

> +
> +# Main loop.
> +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \
> +     "" ""
> +
> +# All done.
> +dg-finish
> diff --git 
> a/gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c 
> b/gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
> new file mode 100644
> index 00000000000..a8712d949f0
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target { aarch64*-*-linux* && { ! 
> gas_has_build_attributes } } } } */
> +/* { dg-options "-mbranch-protection=bti -dA" } */
> +
> +int main()
> +{
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-not "\.aeabi_subsection 
> \.aeabi-feature-and-bits, 1, 0" } } */
> +/* { dg-final { scan-assembler-not "\.aeabi_attribute 1, 1\t\/\/ 
> Tag_Feature_BTI" } } */

For -not tests, it probably makes sense to test only for:

/* { dg-final { scan-assembler-not "\.aeabi_subsection " } } */
/* { dg-final { scan-assembler-not "\.aeabi_attribute " } } */

That is, we want to avoid emitting these directives at all, rather than
avoid emitting specific arguments/operands.

Thanks,
Richard

> +/* { dg-final { scan-assembler "\.section\t\.note\.gnu\.property" } } */
> +/* { dg-final { scan-assembler "\.word\t1\t\/\/ 
> GNU_PROPERTY_AARCH64_FEATURE_1_AND \\(BTI\\)" } } */
> \ No newline at end of file

Reply via email to