[v1,4/4] aarch64: encapsulate note.gnu.property emission into a class

Message ID 20240927144755.632627-5-matthieu.longo@arm.com
State New
Headers
Series aarch64: add minimal support for GCS build attributes |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Matthieu Longo Sept. 27, 2024, 2:47 p.m. UTC
  gcc/ChangeLog:

    * config.gcc: Add aarch64-dwarf-metadata.o to extra_objs.
    * config/aarch64/aarch64-dwarf-metadata.h (class section_note_gnu_property):
    Encapsulate GNU properties code into a class.
    * config/aarch64/aarch64.cc
    (GNU_PROPERTY_AARCH64_FEATURE_1_AND): Define.
    (GNU_PROPERTY_AARCH64_FEATURE_1_BTI): Likewise.
    (GNU_PROPERTY_AARCH64_FEATURE_1_PAC): Likewise.
    (GNU_PROPERTY_AARCH64_FEATURE_1_GCS): Likewise.
    (aarch64_file_end_indicate_exec_stack): Move GNU properties code to
    aarch64-dwarf-metadata.cc
    * config/aarch64/t-aarch64: Declare target aarch64-dwarf-metadata.o
    * config/aarch64/aarch64-dwarf-metadata.cc: New file.
---
 gcc/config.gcc                               |   2 +-
 gcc/config/aarch64/aarch64-dwarf-metadata.cc | 120 +++++++++++++++++++
 gcc/config/aarch64/aarch64-dwarf-metadata.h  |  19 +++
 gcc/config/aarch64/aarch64.cc                |  87 +-------------
 gcc/config/aarch64/t-aarch64                 |   7 ++
 5 files changed, 153 insertions(+), 82 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc
  

Comments

Richard Sandiford Oct. 8, 2024, 5:51 p.m. UTC | #1
Matthieu Longo <matthieu.longo@arm.com> writes:
> gcc/ChangeLog:
>
>     * config.gcc: Add aarch64-dwarf-metadata.o to extra_objs.
>     * config/aarch64/aarch64-dwarf-metadata.h (class section_note_gnu_property):
>     Encapsulate GNU properties code into a class.
>     * config/aarch64/aarch64.cc
>     (GNU_PROPERTY_AARCH64_FEATURE_1_AND): Define.
>     (GNU_PROPERTY_AARCH64_FEATURE_1_BTI): Likewise.
>     (GNU_PROPERTY_AARCH64_FEATURE_1_PAC): Likewise.
>     (GNU_PROPERTY_AARCH64_FEATURE_1_GCS): Likewise.
>     (aarch64_file_end_indicate_exec_stack): Move GNU properties code to
>     aarch64-dwarf-metadata.cc
>     * config/aarch64/t-aarch64: Declare target aarch64-dwarf-metadata.o
>     * config/aarch64/aarch64-dwarf-metadata.cc: New file.
> ---
>  gcc/config.gcc                               |   2 +-
>  gcc/config/aarch64/aarch64-dwarf-metadata.cc | 120 +++++++++++++++++++
>  gcc/config/aarch64/aarch64-dwarf-metadata.h  |  19 +++
>  gcc/config/aarch64/aarch64.cc                |  87 +-------------
>  gcc/config/aarch64/t-aarch64                 |   7 ++
>  5 files changed, 153 insertions(+), 82 deletions(-)
>  create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc
>
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index f09ce9f63a0..b448c2a91d1 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -351,7 +351,7 @@ aarch64*-*-*)
>  	c_target_objs="aarch64-c.o"
>  	cxx_target_objs="aarch64-c.o"
>  	d_target_objs="aarch64-d.o"
> -	extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o aarch64-early-ra.o aarch64-ldp-fusion.o"
> +	extra_objs="aarch64-builtins.o aarch-common.o aarch64-dwarf-metadata.o aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o aarch64-early-ra.o aarch64-ldp-fusion.o"
>  	target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.h \$(srcdir)/config/aarch64/aarch64-builtins.cc \$(srcdir)/config/aarch64/aarch64-sve-builtins.h \$(srcdir)/config/aarch64/aarch64-sve-builtins.cc"
>  	target_has_targetm_common=yes
>  	;;
> diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.cc b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
> new file mode 100644
> index 00000000000..36659862b59
> --- /dev/null
> +++ b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
> @@ -0,0 +1,120 @@

Missing copyright & licence.

> +#define INCLUDE_STRING
> +#define INCLUDE_ALGORITHM
> +#define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "backend.h"
> +#include "target.h"
> +#include "rtl.h"
> +#include "output.h"
> +
> +#include "aarch64-dwarf-metadata.h"
> +
> [...]
> diff --git a/gcc/config/aarch64/t-aarch64 b/gcc/config/aarch64/t-aarch64
> index c2a0715e9ab..194e3a4ac99 100644
> --- a/gcc/config/aarch64/t-aarch64
> +++ b/gcc/config/aarch64/t-aarch64
> @@ -139,6 +139,13 @@ aarch-common.o: $(srcdir)/config/arm/aarch-common.cc $(CONFIG_H) $(SYSTEM_H) \
>  	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
>  		$(srcdir)/config/arm/aarch-common.cc
>  
> +aarch64-dwarf-metadata.o: $(srcdir)/config/aarch64/aarch64-dwarf-metadata.cc \
> +    $(CONFIG_H) \
> +    $(SYSTEM_H) \
> +    $(TARGET_H)

Looks like this also needs: $(RTL_H) output.h aarch64-dwarf-metadata.h

The comments for patch 1 apply to this refactored version too, but otherwise
it looks good.

Thanks,
Richard


> +	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_SPPFLAGS) $(INCLUDES) \
> +	  $(srcdir)/config/aarch64/aarch64-dwarf-metadata.cc
> +
>  aarch64-c.o: $(srcdir)/config/aarch64/aarch64-c.cc $(CONFIG_H) $(SYSTEM_H) \
>      coretypes.h $(TM_H) $(TREE_H) output.h $(C_COMMON_H) $(TARGET_H)
>  	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
  

Patch

diff --git a/gcc/config.gcc b/gcc/config.gcc
index f09ce9f63a0..b448c2a91d1 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -351,7 +351,7 @@  aarch64*-*-*)
 	c_target_objs="aarch64-c.o"
 	cxx_target_objs="aarch64-c.o"
 	d_target_objs="aarch64-d.o"
-	extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o aarch64-early-ra.o aarch64-ldp-fusion.o"
+	extra_objs="aarch64-builtins.o aarch-common.o aarch64-dwarf-metadata.o aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o aarch64-early-ra.o aarch64-ldp-fusion.o"
 	target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.h \$(srcdir)/config/aarch64/aarch64-builtins.cc \$(srcdir)/config/aarch64/aarch64-sve-builtins.h \$(srcdir)/config/aarch64/aarch64-sve-builtins.cc"
 	target_has_targetm_common=yes
 	;;
diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.cc b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
new file mode 100644
index 00000000000..36659862b59
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
@@ -0,0 +1,120 @@ 
+#define INCLUDE_STRING
+#define INCLUDE_ALGORITHM
+#define INCLUDE_MEMORY
+#define INCLUDE_VECTOR
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "target.h"
+#include "rtl.h"
+#include "output.h"
+
+#include "aarch64-dwarf-metadata.h"
+
+#include <numeric>
+
+/* Defined for convenience.  */
+#define POINTER_BYTES (POINTER_SIZE / BITS_PER_UNIT)
+
+namespace aarch64 {
+
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_AND = 0xc0000000;
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_BTI = (1U << 0);
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_PAC = (1U << 1);
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_GCS = (1U << 2);
+
+namespace {
+
+std::string join_s (std::string s1, const std::string &s2)
+{
+  constexpr const char* separator = ", ";
+  return std::move (s1)
+   .append (separator)
+   .append (s2);
+};
+
+std::string gnu_property_features_to_string (unsigned feature_1_and)
+{
+  std::vector<std::string> feature_bits;
+
+  if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
+    feature_bits.push_back ("BTI");
+  if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_PAC)
+    feature_bits.push_back ("PAC");
+  if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_GCS)
+    feature_bits.push_back ("GCS");
+
+  if (feature_bits.empty ())
+    return {};
+
+  return std::accumulate(std::next(feature_bits.cbegin()), feature_bits.cend(),
+			 feature_bits[0], join_s);
+};
+
+} // namespace anonymous
+
+section_note_gnu_property::section_note_gnu_property():
+  feature_1_and(0)
+{}
+
+void section_note_gnu_property::bti_enabled()
+{
+  feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+}
+
+void section_note_gnu_property::pac_enabled()
+{
+  feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+}
+
+void section_note_gnu_property::gcs_enabled()
+{
+  feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_GCS;
+}
+
+void section_note_gnu_property::write () const
+{
+  if (feature_1_and)
+    {
+      /* Generate .note.gnu.property section.  */
+      switch_to_section (get_section (".note.gnu.property",
+				      SECTION_NOTYPE, NULL));
+
+      /* PT_NOTE header: namesz, descsz, type.
+	 namesz = 4 ("GNU\0")
+	 descsz = 16 (Size of the program property array)
+		  [(12 + padding) * Number of array elements]
+	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
+      assemble_align (POINTER_SIZE);
+      assemble_integer (GEN_INT (4), 4, 32, 1);
+      assemble_integer (GEN_INT (ROUND_UP (12, POINTER_BYTES)), 4, 32, 1);
+      assemble_integer (GEN_INT (5), 4, 32, 1);
+
+      /* PT_NOTE name.  */
+      assemble_string ("GNU", 4);
+
+      /* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
+	 type   = GNU_PROPERTY_AARCH64_FEATURE_1_AND
+	 datasz = 4
+	 data   = feature_1_and.  */
+      assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 1);
+      assemble_integer (GEN_INT (4), 4, 32, 1);
+
+      if (!flag_debug_asm)
+	assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
+      else
+	{
+	  asm_fprintf (asm_out_file, "\t.word\t%u", feature_1_and);
+	  auto const& s_features
+	    = gnu_property_features_to_string (feature_1_and);
+	  asm_fprintf (asm_out_file,
+	    "\t%s GNU_PROPERTY_AARCH64_FEATURE_1_AND (%s)\n",
+	    ASM_COMMENT_START, s_features.c_str ());
+	}
+      /* Pad the size of the note to the required alignment.  */
+      assemble_align (POINTER_SIZE);
+    }
+}
+
+} // namespace aarch64
\ No newline at end of file
diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.h b/gcc/config/aarch64/aarch64-dwarf-metadata.h
index 9638bc7702f..006baa502f5 100644
--- a/gcc/config/aarch64/aarch64-dwarf-metadata.h
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.h
@@ -242,6 +242,25 @@  class aeabi_subsection
     auto_vec<aeabi_attribute<T_tag, T_val>, N> attributes_;
 };
 
+class section_note_gnu_property
+{
+  public:
+    section_note_gnu_property();
+
+    /* Add BTI flag to GNU properties.  */
+    void bti_enabled();
+    /* Add GCS flag to GNU properties.  */
+    void gcs_enabled();
+    /* Add PAC flag to GNU properties.  */
+    void pac_enabled();
+
+    /* Write the data to the assembly file.  */
+    void write () const;
+
+  private:
+    unsigned feature_1_and;
+};
+
 } // namespace aarch64
 
 #endif /* GCC_AARCH64_DWARF_METADATA_H */
\ No newline at end of file
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 0cc8a5ee9ad..80af44d9a8f 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -99,8 +99,6 @@ 
 #include "ipa-fnsummary.h"
 #include "hash-map.h"
 
-#include <numeric>
-
 /* This file should be included last.  */
 #include "target-def.h"
 
@@ -29105,97 +29103,24 @@  aarch64_can_tag_addresses ()
 
 /* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
    section at the end if needed.  */
-#define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc0000000
-#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI	(1U << 0)
-#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC	(1U << 1)
-#define GNU_PROPERTY_AARCH64_FEATURE_1_GCS	(1U << 2)
 void
 aarch64_file_end_indicate_exec_stack ()
 {
   file_end_indicate_exec_stack ();
 
-  unsigned feature_1_and = 0;
+  aarch64::section_note_gnu_property gnu_properties;
+
   if (aarch_bti_enabled ())
-    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+    gnu_properties.bti_enabled ();
 
   if (aarch_ra_sign_scope != AARCH_FUNCTION_NONE)
-    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+    gnu_properties.pac_enabled ();
 
   if (aarch64_gcs_enabled ())
-    feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_GCS;
-
-  if (feature_1_and)
-    {
-      /* Generate .note.gnu.property section.  */
-      switch_to_section (get_section (".note.gnu.property",
-				      SECTION_NOTYPE, NULL));
+    gnu_properties.gcs_enabled ();
 
-      /* PT_NOTE header: namesz, descsz, type.
-	 namesz = 4 ("GNU\0")
-	 descsz = 16 (Size of the program property array)
-		  [(12 + padding) * Number of array elements]
-	 type   = 5 (NT_GNU_PROPERTY_TYPE_0).  */
-      assemble_align (POINTER_SIZE);
-      assemble_integer (GEN_INT (4), 4, 32, 1);
-      assemble_integer (GEN_INT (ROUND_UP (12, POINTER_BYTES)), 4, 32, 1);
-      assemble_integer (GEN_INT (5), 4, 32, 1);
-
-      /* PT_NOTE name.  */
-      assemble_string ("GNU", 4);
-
-      /* PT_NOTE contents for NT_GNU_PROPERTY_TYPE_0:
-	 type   = GNU_PROPERTY_AARCH64_FEATURE_1_AND
-	 datasz = 4
-	 data   = feature_1_and.  */
-      assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 1);
-      assemble_integer (GEN_INT (4), 4, 32, 1);
-
-      if (!flag_debug_asm)
-	assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
-      else
-	{
-	  asm_fprintf (asm_out_file, "\t.word\t%u", feature_1_and);
-
-	  auto join_s = [] (std::string s1,
-			    const std::string &s2,
-			    const std::string &separator = ", ") -> std::string
-	  {
-	    return std::move (s1)
-	      .append (separator)
-	      .append (s2);
-	  };
-
-	  auto features_to_string
-	    = [&join_s] (unsigned feature_1_and) -> std::string
-	  {
-	    std::vector<std::string> feature_bits;
-	    if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
-	      feature_bits.push_back ("BTI");
-	    if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_PAC)
-	      feature_bits.push_back ("PAC");
-	    if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_GCS)
-	      feature_bits.push_back ("GCS");
-
-	    if (feature_bits.empty ())
-	      return {};
-	    return std::accumulate(std::next(feature_bits.cbegin()),
-				   feature_bits.cend(),
-				   feature_bits[0],
-				   join_s);
-	  };
-	  auto const& s_features = features_to_string (feature_1_and);
-	  asm_fprintf (asm_out_file,
-	    "\t%s GNU_PROPERTY_AARCH64_FEATURE_1_AND (%s)\n",
-	    ASM_COMMENT_START, s_features.c_str ());
-	}
-      /* Pad the size of the note to the required alignment.  */
-      assemble_align (POINTER_SIZE);
-    }
+  gnu_properties.write ();
 }
-#undef GNU_PROPERTY_AARCH64_FEATURE_1_GCS
-#undef GNU_PROPERTY_AARCH64_FEATURE_1_PAC
-#undef GNU_PROPERTY_AARCH64_FEATURE_1_BTI
-#undef GNU_PROPERTY_AARCH64_FEATURE_1_AND
 
 /* Helper function for straight line speculation.
    Return what barrier should be emitted for straight line speculation
diff --git a/gcc/config/aarch64/t-aarch64 b/gcc/config/aarch64/t-aarch64
index c2a0715e9ab..194e3a4ac99 100644
--- a/gcc/config/aarch64/t-aarch64
+++ b/gcc/config/aarch64/t-aarch64
@@ -139,6 +139,13 @@  aarch-common.o: $(srcdir)/config/arm/aarch-common.cc $(CONFIG_H) $(SYSTEM_H) \
 	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
 		$(srcdir)/config/arm/aarch-common.cc
 
+aarch64-dwarf-metadata.o: $(srcdir)/config/aarch64/aarch64-dwarf-metadata.cc \
+    $(CONFIG_H) \
+    $(SYSTEM_H) \
+    $(TARGET_H)
+	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_SPPFLAGS) $(INCLUDES) \
+	  $(srcdir)/config/aarch64/aarch64-dwarf-metadata.cc
+
 aarch64-c.o: $(srcdir)/config/aarch64/aarch64-c.cc $(CONFIG_H) $(SYSTEM_H) \
     coretypes.h $(TM_H) $(TREE_H) output.h $(C_COMMON_H) $(TARGET_H)
 	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \