On 2024-10-08 18:42, Richard Sandiford wrote:
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?
The binutils interface is agnostic of the tag value. binutils only
checks whether the values in this object are matching in the other
object, that's all.
Checking for the support of each feature in Gas does not make sense.
As you recommended, I defined and used HAVE_AS_BUILD_AEABI_ATTRIBUTE in
the next revision.
[...]
@@ -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.
This check comes from the initial revision
(https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662825.html).
I am not sure why there was this restriction, it does not make sense to
me as well.
I removed it in the next revision.
+ /* Check the current assembly supports gcs build attributes, if not
"Check whether the current assembler supports..."
Fixed.
+ 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.
Done. I added it at the top of aarch64.cc in the next revision.
+ 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...".
I renamed the function to
check_effective_target_aarch64_gas_has_build_attributes to make it clear
that this is for AArch64 targets, and moved the code into
lib/target-supports.exp.
+
+# 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.
Fixed in the next revision.
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
