Matthieu Longo <matthieu.lo...@arm.com> writes: > The previous implementation to emit AEABI build attributes did not > support string values (asciz) in aeabi_subsection, and was not > emitting values associated to tags in the assembly comments. > > This new approach provides a more user-friendly interface relying on > typing, and improves the emitted assembly comments: > * aeabi_attribute: > ** Adds the interpreted value next to the tag in the assembly > comment. > ** Supports asciz values. > * aeabi_subsection: > ** Adds debug information for its parameters. > ** Auto-detects the attribute types when declaring the subsection. > > Additionally, it is also interesting to note that the code was moved > to a separate file to improve modularity and "releave" the 1000-lines
I think you dropped a 0. I wish it was only 1000 :-) > long aarch64.cc file from a few lines. Finally, it introduces a new > namespace "aarch64::" for AArch64 backend which reduce the length of > function names by not prepending 'aarch64_' to each of them. > [...] > diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.h > b/gcc/config/aarch64/aarch64-dwarf-metadata.h > new file mode 100644 > index 00000000000..01f08ad073e > --- /dev/null > +++ b/gcc/config/aarch64/aarch64-dwarf-metadata.h > @@ -0,0 +1,226 @@ > +/* DWARF metadata for AArch64 architecture. > + Copyright (C) 2024 Free Software Foundation, Inc. > + Contributed by ARM Ltd. > + > + This file is part of GCC. > + > + GCC 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, or (at your option) > + any later version. > + > + GCC 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/>. */ > + > +#ifndef GCC_AARCH64_DWARF_METADATA_H > +#define GCC_AARCH64_DWARF_METADATA_H > + > +#include "system.h" We should drop this line. It's the .cc file's responsibility to include system.h. > +#include "vec.h" > + > +namespace aarch64 { > + > +enum attr_val_type : uint8_t > +{ > + uleb128 = 0x0, > + asciz = 0x1, > +}; > + > +enum BA_TagFeature_t : uint8_t > +{ > + Tag_Feature_BTI = 1, > + Tag_Feature_PAC = 2, > + Tag_Feature_GCS = 3, > +}; > + > +template <typename T_tag, typename T_val> > +struct aeabi_attribute > +{ > + T_tag tag; > + T_val value; > +}; > + > +template <typename T_tag, typename T_val> > +aeabi_attribute<T_tag, T_val> > +make_aeabi_attribute (T_tag tag, T_val val) > +{ > + return aeabi_attribute<T_tag, T_val>{tag, val}; > +} > + > +namespace details { > + > +constexpr const char * > +to_c_str (bool b) > +{ > + return b ? "true" : "false"; > +} > + > +constexpr const char * > +to_c_str (const char *s) > +{ > + return s; > +} > + > +constexpr const char * > +to_c_str (attr_val_type t) > +{ > + return (t == uleb128 ? "ULEB128" > + : t == asciz ? "asciz" > + : nullptr); > +} > + > +constexpr const char * > +to_c_str (BA_TagFeature_t feature) > +{ > + return (feature == Tag_Feature_BTI ? "Tag_Feature_BTI" > + : feature == Tag_Feature_PAC ? "Tag_Feature_PAC" > + : feature == Tag_Feature_GCS ? "Tag_Feature_GCS" > + : nullptr); > +} > + > +template < > + typename T, > + typename = typename std::enable_if<std::is_unsigned<T>::value, T>::type > +> > +constexpr const char * > +aeabi_attr_str_fmt (T phantom __attribute__((unused))) FWIW, it would be ok to drop the parameter name and the attribute. But it's ok as-is too, if you think it makes the intention clearer. > +{ > + return "\t.aeabi_attribute %u, %u"; > +} > + > +constexpr const char * > +aeabi_attr_str_fmt (const char *phantom __attribute__((unused))) > +{ > + return "\t.aeabi_attribute %u, \"%s\""; > +} > [...] > @@ -24834,17 +24808,21 @@ aarch64_start_file (void) > asm_fprintf (asm_out_file, "\t.arch %s\n", > aarch64_last_printed_arch_string.c_str ()); > > - /* Check whether the current assembly supports gcs build attributes, if not > - fallback to .note.gnu.property section. */ > + /* Check whether the current assembler supports AEABI build attributes, if > + not fallback to .note.gnu.property section. */ > #if (HAVE_AS_AEABI_BUILD_ATTRIBUTES) Just to note that, as with patch 2, I hope this could be: if (HAVE_AS_AEABI_BUILD_ATTRIBUTES) { ... } instead. OK with those changes, thanks. Richard
