[PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[Andrew Carlotti]

This series of patches fixes issues with some intrinsics being incorrectly
gated by global target options, instad of just using function-specific target
options.  These issues have been present since the +tme, +memtag and +ls64
intrinsics were introduced.

Compared to the previous version, this series no longer adds feature checks to
the intrinsic expanders, and fixes various formatting issues pointed out by
Richard Sandiford.

Additionally, the series now refactors the checking of TARGET_GENERAL_REGS_ONLY
in check_required_extensions.  This refactor is included as a new patch (1/5)
to make the diffs more readable.


Bootstrapped and regression tested on aarch64.  Ok to merge?

Also, ok for backports to affected versions (with regression tests)?

[PATCH v3 1/5] aarch64: Refactor check_required_extensions

[Andrew Carlotti]

Replace TARGET_GENERAL_REGS_ONLY check with an explicit check that
aarch64_isa_flags enables all required extensions.  This will be more
flexible when repurposing this function for non-SVE intrinsics.

gcc/ChangeLog:

	* config/aarch64/aarch64-sve-builtins.cc
	(check_required_registers): Remove target check and rename to...
	(report_missing_registers): ...this.
	(check_required_extensions): Refactor.


diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index 0a560eaedca14832bfacef3225bd467691e16e99..1fe380dd1efb953466fd902f86eef8938059a261 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -1094,27 +1094,19 @@ report_missing_extension (location_t location, tree fndecl,
   reported_missing_extension_p = true;
 }
 
-/* Check whether the registers required by SVE function fndecl are available.
-   Report an error against LOCATION and return false if not.  */
-static bool
-check_required_registers (location_t location, tree fndecl)
+/* Report an error against LOCATION that the user has tried to use
+   function FNDECL when non-general registers are disabled.  */
+static void
+report_missing_registers (location_t location, tree fndecl)
 {
   /* Avoid reporting a slew of messages for a single oversight.  */
   if (reported_missing_registers_p)
-    return false;
-
-  if (TARGET_GENERAL_REGS_ONLY)
-    {
-      /* SVE registers are not usable when -mgeneral-regs-only option
-	 is specified.  */
-      error_at (location,
-		"ACLE function %qD is incompatible with the use of %qs",
-		fndecl, "-mgeneral-regs-only");
-      reported_missing_registers_p = true;
-      return false;
-    }
+    return;
 
-  return true;
+  error_at (location,
+	    "ACLE function %qD is incompatible with the use of %qs",
+	    fndecl, "-mgeneral-regs-only");
+  reported_missing_registers_p = true;
 }
 
 /* Check whether all the AARCH64_FL_* values in REQUIRED_EXTENSIONS are
@@ -1124,9 +1116,19 @@ static bool
 check_required_extensions (location_t location, tree fndecl,
 			   aarch64_feature_flags required_extensions)
 {
+  if ((required_extensions & ~aarch64_isa_flags) == 0)
+    return true;
+
   auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
+
   if (missing_extensions == 0)
-    return check_required_registers (location, fndecl);
+    {
+      /* All required extensions are enabled in aarch64_asm_isa_flags, so the
+	 error must be the use of general-regs-only.  */
+      report_missing_registers (location, fndecl);
+      return false;
+    }
+
 
   if (missing_extensions & AARCH64_FL_SM_OFF)
     {

[PATCH v3 2/5] aarch64: Move check_required_extensions

[Andrew Carlotti]

Move SVE extension checking functionality to aarch64-builtins.cc, so
that it can be shared by non-SVE intrinsics.

gcc/ChangeLog:

	* config/aarch64/aarch64-sve-builtins.cc (check_builtin_call)
	(expand_builtin): Update calls to the below.
	(report_missing_extension, report_missing_registers)
	(check_required_extensions): Move out of aarch64_sve namespace,
	rename, and move into...
	* config/aarch64/aarch64-builtins.cc (aarch64_report_missing_extension)
	(aarch64_report_missing_registers)
	(aarch64_check_required_extensions) ...here.
	* config/aarch64/aarch64-protos.h (aarch64_check_required_extensions):
	Add prototype.


diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
index 30669f8aa1823b64689c67e306d38e234bd31698..a07adcee6e266c947855041ed7432085f6448836 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -2180,6 +2180,106 @@ aarch64_general_builtin_decl (unsigned code, bool)
   return aarch64_builtin_decls[code];
 }
 
+/* True if we've already complained about attempts to use functions
+   when the required extension is disabled.  */
+static bool reported_missing_extension_p;
+
+/* True if we've already complained about attempts to use functions
+   which require registers that are missing.  */
+static bool reported_missing_registers_p;
+
+/* Report an error against LOCATION that the user has tried to use
+   function FNDECL when extension EXTENSION is disabled.  */
+static void
+aarch64_report_missing_extension (location_t location, tree fndecl,
+				  const char *extension)
+{
+  /* Avoid reporting a slew of messages for a single oversight.  */
+  if (reported_missing_extension_p)
+    return;
+
+  error_at (location, "ACLE function %qD requires ISA extension %qs",
+	    fndecl, extension);
+  inform (location, "you can enable %qs using the command-line"
+	  " option %<-march%>, or by using the %<target%>"
+	  " attribute or pragma", extension);
+  reported_missing_extension_p = true;
+}
+
+/* Report an error against LOCATION that the user has tried to use
+   function FNDECL when non-general registers are disabled.  */
+static void
+aarch64_report_missing_registers (location_t location, tree fndecl)
+{
+  /* Avoid reporting a slew of messages for a single oversight.  */
+  if (reported_missing_registers_p)
+    return;
+
+  error_at (location,
+	    "ACLE function %qD is incompatible with the use of %qs",
+	    fndecl, "-mgeneral-regs-only");
+  reported_missing_registers_p = true;
+}
+
+/* Check whether all the AARCH64_FL_* values in REQUIRED_EXTENSIONS are
+   enabled, given that those extensions are required for function FNDECL.
+   Report an error against LOCATION if not.  */
+bool
+aarch64_check_required_extensions (location_t location, tree fndecl,
+				   aarch64_feature_flags required_extensions)
+{
+  if ((required_extensions & ~aarch64_isa_flags) == 0)
+    return true;
+
+  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
+
+  if (missing_extensions == 0)
+    {
+      /* All required extensions are enabled in aarch64_asm_isa_flags, so the
+	 error must be the use of general-regs-only.  */
+      aarch64_report_missing_registers (location, fndecl);
+      return false;
+    }
+
+  if (missing_extensions & AARCH64_FL_SM_OFF)
+    {
+      error_at (location, "ACLE function %qD cannot be called when"
+		" SME streaming mode is enabled", fndecl);
+      return false;
+    }
+
+  if (missing_extensions & AARCH64_FL_SM_ON)
+    {
+      error_at (location, "ACLE function %qD can only be called when"
+		" SME streaming mode is enabled", fndecl);
+      return false;
+    }
+
+  if (missing_extensions & AARCH64_FL_ZA_ON)
+    {
+      error_at (location, "ACLE function %qD can only be called from"
+		" a function that has %qs state", fndecl, "za");
+      return false;
+    }
+
+  static const struct {
+    aarch64_feature_flags flag;
+    const char *name;
+  } extensions[] = {
+#define AARCH64_OPT_EXTENSION(EXT_NAME, IDENT, C, D, E, F) \
+    { AARCH64_FL_##IDENT, EXT_NAME },
+#include "aarch64-option-extensions.def"
+  };
+
+  for (unsigned int i = 0; i < ARRAY_SIZE (extensions); ++i)
+    if (missing_extensions & extensions[i].flag)
+      {
+	aarch64_report_missing_extension (location, fndecl, extensions[i].name);
+	return false;
+      }
+  gcc_unreachable ();
+}
+
 bool
 aarch64_general_check_builtin_call (location_t location, vec<location_t>,
 			    unsigned int code, tree fndecl,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index f64afe2889018e1c4735a1677e6bf5febc4a7665..28613c425188cd270d9a2deeb91ae61b29aa1f07 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -1008,6 +1008,8 @@ tree aarch64_general_builtin_rsqrt (unsigned int);
 void handle_arm_acle_h (void);
 void handle_arm_neon_h (void);
 
+bool aarch64_check_required_extensions (location_t, tree,
+					aarch64_feature_flags);
 bool aarch64_general_check_builtin_call (location_t, vec<location_t>,
 					 unsigned int, tree, unsigned int,
 					 tree *);
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index 1fe380dd1efb953466fd902f86eef8938059a261..5ca9ec32b691fd53733b01c52ad7a25cc5de9b93 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -947,14 +947,6 @@ static hash_table<registered_function_hasher> *function_table;
    are IDENTIFIER_NODEs.  */
 static GTY(()) hash_map<tree, registered_function *> *overload_names[2];
 
-/* True if we've already complained about attempts to use functions
-   when the required extension is disabled.  */
-static bool reported_missing_extension_p;
-
-/* True if we've already complained about attempts to use functions
-   which require registers that are missing.  */
-static bool reported_missing_registers_p;
-
 /* Record that TYPE is an ABI-defined SVE type that contains NUM_ZR SVE vectors
    and NUM_PR SVE predicates.  MANGLED_NAME, if nonnull, is the ABI-defined
    mangling of the type.  ACLE_NAME is the <arm_sve.h> name of the type.  */
@@ -1076,98 +1068,6 @@ lookup_fndecl (tree fndecl)
   return &(*registered_functions)[subcode]->instance;
 }
 
-/* Report an error against LOCATION that the user has tried to use
-   function FNDECL when extension EXTENSION is disabled.  */
-static void
-report_missing_extension (location_t location, tree fndecl,
-			  const char *extension)
-{
-  /* Avoid reporting a slew of messages for a single oversight.  */
-  if (reported_missing_extension_p)
-    return;
-
-  error_at (location, "ACLE function %qD requires ISA extension %qs",
-	    fndecl, extension);
-  inform (location, "you can enable %qs using the command-line"
-	  " option %<-march%>, or by using the %<target%>"
-	  " attribute or pragma", extension);
-  reported_missing_extension_p = true;
-}
-
-/* Report an error against LOCATION that the user has tried to use
-   function FNDECL when non-general registers are disabled.  */
-static void
-report_missing_registers (location_t location, tree fndecl)
-{
-  /* Avoid reporting a slew of messages for a single oversight.  */
-  if (reported_missing_registers_p)
-    return;
-
-  error_at (location,
-	    "ACLE function %qD is incompatible with the use of %qs",
-	    fndecl, "-mgeneral-regs-only");
-  reported_missing_registers_p = true;
-}
-
-/* Check whether all the AARCH64_FL_* values in REQUIRED_EXTENSIONS are
-   enabled, given that those extensions are required for function FNDECL.
-   Report an error against LOCATION if not.  */
-static bool
-check_required_extensions (location_t location, tree fndecl,
-			   aarch64_feature_flags required_extensions)
-{
-  if ((required_extensions & ~aarch64_isa_flags) == 0)
-    return true;
-
-  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
-
-  if (missing_extensions == 0)
-    {
-      /* All required extensions are enabled in aarch64_asm_isa_flags, so the
-	 error must be the use of general-regs-only.  */
-      report_missing_registers (location, fndecl);
-      return false;
-    }
-
-
-  if (missing_extensions & AARCH64_FL_SM_OFF)
-    {
-      error_at (location, "ACLE function %qD cannot be called when"
-		" SME streaming mode is enabled", fndecl);
-      return false;
-    }
-
-  if (missing_extensions & AARCH64_FL_SM_ON)
-    {
-      error_at (location, "ACLE function %qD can only be called when"
-		" SME streaming mode is enabled", fndecl);
-      return false;
-    }
-
-  if (missing_extensions & AARCH64_FL_ZA_ON)
-    {
-      error_at (location, "ACLE function %qD can only be called from"
-		" a function that has %qs state", fndecl, "za");
-      return false;
-    }
-
-  static const struct {
-    aarch64_feature_flags flag;
-    const char *name;
-  } extensions[] = {
-#define AARCH64_OPT_EXTENSION(EXT_NAME, IDENT, C, D, E, F) \
-    { AARCH64_FL_##IDENT, EXT_NAME },
-#include "aarch64-option-extensions.def"
-  };
-
-  for (unsigned int i = 0; i < ARRAY_SIZE (extensions); ++i)
-    if (missing_extensions & extensions[i].flag)
-      {
-	report_missing_extension (location, fndecl, extensions[i].name);
-	return false;
-      }
-  gcc_unreachable ();
-}
 
 /* Report that LOCATION has a call to FNDECL in which argument ARGNO
    was not an integer constant expression.  ARGNO counts from zero.  */
@@ -4763,7 +4663,8 @@ check_builtin_call (location_t location, vec<location_t>, unsigned int code,
 		    tree fndecl, unsigned int nargs, tree *args)
 {
   const registered_function &rfn = *(*registered_functions)[code];
-  if (!check_required_extensions (location, rfn.decl, rfn.required_extensions))
+  if (!aarch64_check_required_extensions (location, rfn.decl,
+					  rfn.required_extensions))
     return false;
   return function_checker (location, rfn.instance, fndecl,
 			   TREE_TYPE (rfn.decl), nargs, args).check ();
@@ -4786,8 +4687,8 @@ rtx
 expand_builtin (unsigned int code, tree exp, rtx target)
 {
   registered_function &rfn = *(*registered_functions)[code];
-  if (!check_required_extensions (EXPR_LOCATION (exp), rfn.decl,
-				  rfn.required_extensions))
+  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), rfn.decl,
+					  rfn.required_extensions))
     return target;
   return function_expander (rfn.instance, rfn.decl, exp, target).expand ();
 }

[PATCH v3 3/5] aarch64: Fix tme intrinsic availability

[Andrew Carlotti]

The availability of tme intrinsics was previously gated at both
initialisation time (using global target options) and usage time
(accounting for function-specific target options).  This patch removes
the check at initialisation time, and also moves the intrinsics out of
the header file to allow for better error messages (matching the
existing error messages for SVE intrinsics).

gcc/ChangeLog:

	PR target/112108
	* config/aarch64/aarch64-builtins.cc (aarch64_init_tme_builtins):
	(aarch64_general_init_builtins): Move tme initialisation...
	(handle_arm_acle_h): ...to here, and remove feature check.
	(aarch64_general_check_builtin_call): Check tme intrinsics.
	* config/aarch64/arm_acle.h (__tstart, __tcommit, __tcancel)
	(__ttest): Remove.
	(_TMFAILURE_*): Define unconditionally.

gcc/testsuite/ChangeLog:

	PR target/112108
	* gcc.target/aarch64/acle/tme_guard-1.c: New test.
	* gcc.target/aarch64/acle/tme_guard-2.c: New test.
	* gcc.target/aarch64/acle/tme_guard-3.c: New test.
	* gcc.target/aarch64/acle/tme_guard-4.c: New test.


diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
index a07adcee6e266c947855041ed7432085f6448836..60e4c217921bc1144bfa436a168a4a1dc194f44e 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -1791,21 +1791,17 @@ aarch64_init_tme_builtins (void)
     = build_function_type_list (void_type_node, uint64_type_node, NULL);
 
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TSTART]
-    = aarch64_general_add_builtin ("__builtin_aarch64_tstart",
-				   ftype_uint64_void,
-				   AARCH64_TME_BUILTIN_TSTART);
+    = aarch64_general_simulate_builtin ("__tstart", ftype_uint64_void,
+					AARCH64_TME_BUILTIN_TSTART);
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TTEST]
-    = aarch64_general_add_builtin ("__builtin_aarch64_ttest",
-				   ftype_uint64_void,
-				   AARCH64_TME_BUILTIN_TTEST);
+    = aarch64_general_simulate_builtin ("__ttest", ftype_uint64_void,
+					AARCH64_TME_BUILTIN_TTEST);
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCOMMIT]
-    = aarch64_general_add_builtin ("__builtin_aarch64_tcommit",
-				   ftype_void_void,
-				   AARCH64_TME_BUILTIN_TCOMMIT);
+    = aarch64_general_simulate_builtin ("__tcommit", ftype_void_void,
+					AARCH64_TME_BUILTIN_TCOMMIT);
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCANCEL]
-    = aarch64_general_add_builtin ("__builtin_aarch64_tcancel",
-				   ftype_void_uint64,
-				   AARCH64_TME_BUILTIN_TCANCEL);
+    = aarch64_general_simulate_builtin ("__tcancel", ftype_void_uint64,
+					AARCH64_TME_BUILTIN_TCANCEL);
 }
 
 /* Add builtins for Random Number instructions.  */
@@ -2068,6 +2064,7 @@ handle_arm_acle_h (void)
 {
   if (TARGET_LS64)
     aarch64_init_ls64_builtins ();
+  aarch64_init_tme_builtins ();
 }
 
 /* Initialize fpsr fpcr getters and setters.  */
@@ -2160,9 +2157,6 @@ aarch64_general_init_builtins (void)
   if (!TARGET_ILP32)
     aarch64_init_pauth_hint_builtins ();
 
-  if (TARGET_TME)
-    aarch64_init_tme_builtins ();
-
   if (TARGET_MEMTAG)
     aarch64_init_memtag_builtins ();
 
@@ -2285,6 +2279,7 @@ aarch64_general_check_builtin_call (location_t location, vec<location_t>,
 			    unsigned int code, tree fndecl,
 			    unsigned int nargs ATTRIBUTE_UNUSED, tree *args)
 {
+  tree decl = aarch64_builtin_decls[code];
   switch (code)
     {
     case AARCH64_RSR:
@@ -2297,15 +2292,29 @@ aarch64_general_check_builtin_call (location_t location, vec<location_t>,
     case AARCH64_WSR64:
     case AARCH64_WSRF:
     case AARCH64_WSRF64:
-      tree addr = STRIP_NOPS (args[0]);
-      if (TREE_CODE (TREE_TYPE (addr)) != POINTER_TYPE
-	  || TREE_CODE (addr) != ADDR_EXPR
-	  || TREE_CODE (TREE_OPERAND (addr, 0)) != STRING_CST)
-	{
-	  error_at (location, "first argument to %qD must be a string literal",
-		    fndecl);
-	  return false;
-	}
+      {
+	tree addr = STRIP_NOPS (args[0]);
+	if (TREE_CODE (TREE_TYPE (addr)) != POINTER_TYPE
+	    || TREE_CODE (addr) != ADDR_EXPR
+	    || TREE_CODE (TREE_OPERAND (addr, 0)) != STRING_CST)
+	  {
+	    error_at (location,
+		      "first argument to %qD must be a string literal",
+		      fndecl);
+	    return false;
+	  }
+	break;
+      }
+
+    case AARCH64_TME_BUILTIN_TSTART:
+    case AARCH64_TME_BUILTIN_TCOMMIT:
+    case AARCH64_TME_BUILTIN_TTEST:
+    case AARCH64_TME_BUILTIN_TCANCEL:
+      return aarch64_check_required_extensions (location, decl,
+						AARCH64_FL_TME);
+
+    default:
+      break;
     }
   /* Default behavior.  */
   return true;
diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index 2aa681090fa205449cf1ac63151565f960716189..2d84ab1bd3f3241196727d7a632a155014708081 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -252,10 +252,7 @@ __crc32d (uint32_t __a, uint64_t __b)
 
 #pragma GCC pop_options
 
-#ifdef __ARM_FEATURE_TME
-#pragma GCC push_options
-#pragma GCC target ("+nothing+tme")
-
+/* Constants for TME failure reason.  */
 #define _TMFAILURE_REASON     0x00007fffu
 #define _TMFAILURE_RTRY       0x00008000u
 #define _TMFAILURE_CNCL       0x00010000u
@@ -268,37 +265,6 @@ __crc32d (uint32_t __a, uint64_t __b)
 #define _TMFAILURE_INT        0x00800000u
 #define _TMFAILURE_TRIVIAL    0x01000000u
 
-__extension__ extern __inline uint64_t
-__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
-__tstart (void)
-{
-  return __builtin_aarch64_tstart ();
-}
-
-__extension__ extern __inline void
-__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
-__tcommit (void)
-{
-  __builtin_aarch64_tcommit ();
-}
-
-__extension__ extern __inline void
-__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
-__tcancel (const uint64_t __reason)
-{
-  __builtin_aarch64_tcancel (__reason);
-}
-
-__extension__ extern __inline uint64_t
-__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
-__ttest (void)
-{
-  return __builtin_aarch64_ttest ();
-}
-
-#pragma GCC pop_options
-#endif
-
 #ifdef __ARM_FEATURE_LS64
 typedef __arm_data512_t data512_t;
 #endif
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-1.c b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..9894d3341f6bc352c22ad95d4d1e000207ca8d00
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a" } */
+
+#include <arm_acle.h>
+
+void foo (void)
+{
+  __tcommit (); /* { dg-error {ACLE function '__tcommit' requires ISA extension 'tme'} } */
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-2.c b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..4e3d69712b14a8123f45a2ead02b5048883614d9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a" } */
+
+#include <arm_acle.h>
+
+#pragma GCC target("arch=armv8-a+tme")
+void foo (void)
+{
+  __tcommit ();
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-3.c b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..5f480ebb8209fdaeb4baa77dbdf5467d16936644
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-3.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a+tme -mgeneral-regs-only" } */
+
+#include <arm_acle.h>
+
+void foo (void)
+{
+  __tcommit ();
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-4.c b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-4.c
new file mode 100644
index 0000000000000000000000000000000000000000..bf4d368370c614ffe33035d9ec4f86988f3f1c30
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/tme_guard-4.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a+tme" } */
+
+#include <arm_acle.h>
+
+#pragma GCC target("arch=armv8-a")
+void foo (void)
+{
+  __tcommit (); /* { dg-error {ACLE function '__tcommit' requires ISA extension 'tme'} } */
+}

[PATCH v3 4/5] aarch64: Fix memtag intrinsic availability

[Andrew Carlotti]

The availability of memtag intrinsics and data types were determined
solely by the globally specified architecture features, which did not
reflect any changes specified in target pragmas or attributes.

This patch removes the initialisation-time guards for the intrinsics,
and replaces them with checks at use time. It also removes the macro
indirection from the header file - this simplifies the header, and
allows the missing extension error reporting to find the user-facing
intrinsic names.

gcc/ChangeLog:

	PR target/112108
	* config/aarch64/aarch64-builtins.cc (aarch64_init_memtag_builtins):
	Replace internal builtin names with intrinsic names.
	(aarch64_general_init_builtins): Move memtag intialisation...
	(handle_arm_acle_h): ...to here, and remove feature check.
	(aarch64_general_check_builtin_call): Check memtag intrinsics.
	* config/aarch64/arm_acle.h (__arm_mte_create_random_tag)
	(__arm_mte_exclude_tag, __arm_mte_ptrdiff)
	(__arm_mte_increment_tag, __arm_mte_set_tag, __arm_mte_get_tag):
	Remove.

gcc/testsuite/ChangeLog:

	PR target/112108
	* gcc.target/aarch64/acle/memtag_guard-1.c: New test.
	* gcc.target/aarch64/acle/memtag_guard-2.c: New test.
	* gcc.target/aarch64/acle/memtag_guard-3.c: New test.
	* gcc.target/aarch64/acle/memtag_guard-4.c: New test.


diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
index 60e4c217921bc1144bfa436a168a4a1dc194f44e..9c6d9ec7537e7c473dc42a27a7737d80aab9cddb 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -1932,27 +1932,27 @@ aarch64_init_memtag_builtins (void)
 
 #define AARCH64_INIT_MEMTAG_BUILTINS_DECL(F, N, I, T) \
   aarch64_builtin_decls[AARCH64_MEMTAG_BUILTIN_##F] \
-    = aarch64_general_add_builtin ("__builtin_aarch64_memtag_"#N, \
-				   T, AARCH64_MEMTAG_BUILTIN_##F); \
+    = aarch64_general_simulate_builtin ("__arm_mte_"#N, T, \
+					AARCH64_MEMTAG_BUILTIN_##F); \
   aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
 			      AARCH64_MEMTAG_BUILTIN_START - 1] = \
 				{T, CODE_FOR_##I};
 
   fntype = build_function_type_list (ptr_type_node, ptr_type_node,
 				     uint64_type_node, NULL);
-  AARCH64_INIT_MEMTAG_BUILTINS_DECL (IRG, irg, irg, fntype);
+  AARCH64_INIT_MEMTAG_BUILTINS_DECL (IRG, create_random_tag, irg, fntype);
 
   fntype = build_function_type_list (uint64_type_node, ptr_type_node,
 				     uint64_type_node, NULL);
-  AARCH64_INIT_MEMTAG_BUILTINS_DECL (GMI, gmi, gmi, fntype);
+  AARCH64_INIT_MEMTAG_BUILTINS_DECL (GMI, exclude_tag, gmi, fntype);
 
   fntype = build_function_type_list (ptrdiff_type_node, ptr_type_node,
 				     ptr_type_node, NULL);
-  AARCH64_INIT_MEMTAG_BUILTINS_DECL (SUBP, subp, subp, fntype);
+  AARCH64_INIT_MEMTAG_BUILTINS_DECL (SUBP, ptrdiff, subp, fntype);
 
   fntype = build_function_type_list (ptr_type_node, ptr_type_node,
 				     unsigned_type_node, NULL);
-  AARCH64_INIT_MEMTAG_BUILTINS_DECL (INC_TAG, inc_tag, addg, fntype);
+  AARCH64_INIT_MEMTAG_BUILTINS_DECL (INC_TAG, increment_tag, addg, fntype);
 
   fntype = build_function_type_list (void_type_node, ptr_type_node, NULL);
   AARCH64_INIT_MEMTAG_BUILTINS_DECL (SET_TAG, set_tag, stg, fntype);
@@ -2065,6 +2065,7 @@ handle_arm_acle_h (void)
   if (TARGET_LS64)
     aarch64_init_ls64_builtins ();
   aarch64_init_tme_builtins ();
+  aarch64_init_memtag_builtins ();
 }
 
 /* Initialize fpsr fpcr getters and setters.  */
@@ -2157,9 +2158,6 @@ aarch64_general_init_builtins (void)
   if (!TARGET_ILP32)
     aarch64_init_pauth_hint_builtins ();
 
-  if (TARGET_MEMTAG)
-    aarch64_init_memtag_builtins ();
-
   if (in_lto_p)
     handle_arm_acle_h ();
 }
@@ -2316,7 +2314,12 @@ aarch64_general_check_builtin_call (location_t location, vec<location_t>,
     default:
       break;
     }
-  /* Default behavior.  */
+
+  if (code >= AARCH64_MEMTAG_BUILTIN_START
+      && code <= AARCH64_MEMTAG_BUILTIN_END)
+    return aarch64_check_required_extensions (location, decl,
+					      AARCH64_FL_MEMTAG);
+
   return true;
 }
 
diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index 2d84ab1bd3f3241196727d7a632a155014708081..ab04326791309796125860ce64e63fe858a4a733 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -287,29 +287,6 @@ __rndrrs (uint64_t *__res)
 
 #pragma GCC pop_options
 
-#pragma GCC push_options
-#pragma GCC target ("+nothing+memtag")
-
-#define __arm_mte_create_random_tag(__ptr, __u64_mask) \
-  __builtin_aarch64_memtag_irg(__ptr, __u64_mask)
-
-#define __arm_mte_exclude_tag(__ptr, __u64_excluded) \
-  __builtin_aarch64_memtag_gmi(__ptr, __u64_excluded)
-
-#define __arm_mte_ptrdiff(__ptr_a, __ptr_b) \
-  __builtin_aarch64_memtag_subp(__ptr_a, __ptr_b)
-
-#define __arm_mte_increment_tag(__ptr, __u_offset) \
-  __builtin_aarch64_memtag_inc_tag(__ptr, __u_offset)
-
-#define __arm_mte_set_tag(__tagged_address) \
-  __builtin_aarch64_memtag_set_tag(__tagged_address)
-
-#define __arm_mte_get_tag(__address) \
-  __builtin_aarch64_memtag_get_tag(__address)
-
-#pragma GCC pop_options
-
 #define __arm_rsr(__regname) \
   __builtin_aarch64_rsr (__regname)
 
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-1.c b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..4a34c37f44fae590d2a3f947dd1bf404fe8261fa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.5-a" } */
+
+#include <arm_acle.h>
+
+void foo (int * p)
+{
+  __arm_mte_set_tag (p); /* { dg-error {ACLE function '__arm_mte_set_tag' requires ISA extension 'memtag'} } */
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-2.c b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..bf06b284dadd2082496cfc0a345f7cf5a783bd3b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.5-a" } */
+
+#include <arm_acle.h>
+
+#pragma GCC target("arch=armv8.5-a+memtag")
+void foo (int * p)
+{
+  __arm_mte_set_tag (p);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-3.c b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..1f4ffa454a763e6b35bd84af6ef6c7d6474a518b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-3.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.5-a+memtag -mgeneral-regs-only" } */
+
+#include <arm_acle.h>
+
+void foo (int * p)
+{
+  __arm_mte_set_tag (p);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-4.c b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-4.c
new file mode 100644
index 0000000000000000000000000000000000000000..a1cd45ec79d242dcf710abdc375c29063866eb5a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/memtag_guard-4.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.5-a+memtag" } */
+
+#include <arm_acle.h>
+
+#pragma GCC target("arch=armv8.5-a")
+void foo (int * p)
+{
+  __arm_mte_set_tag (p); /* { dg-error {ACLE function '__arm_mte_set_tag' requires ISA extension 'memtag'} } */
+}

[PATCH v3 5/5] aarch64: Fix ls64 intrinsic availability

[Andrew Carlotti]

The availability of ls64 intrinsics and data types were determined
solely by the globally specified architecture features, which did not
reflect any changes specified in target pragmas or attributes.

This patch removes the initialisation-time guards for the intrinsics,
and replaces them with checks at use time. We also get better error
messages when ls64 is not available (matching the existing error
messages for SVE intrinsics).

The data512_t type is made always available; this is consistent with the
present behaviour for Neon fp16/bf16 types.

gcc/ChangeLog:

	PR target/112108
	* config/aarch64/aarch64-builtins.cc (handle_arm_acle_h): Remove
	feature check at initialisation.
	(aarch64_general_check_builtin_call): Check ls64 intrinsics.
	* config/aarch64/arm_acle.h: (data512_t) Make always available.

gcc/testsuite/ChangeLog:

	PR target/112108
	* gcc.target/aarch64/acle/ls64_guard-1.c: New test.
	* gcc.target/aarch64/acle/ls64_guard-2.c: New test.
	* gcc.target/aarch64/acle/ls64_guard-3.c: New test.
	* gcc.target/aarch64/acle/ls64_guard-4.c: New test.


diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
index 9c6d9ec7537e7c473dc42a27a7737d80aab9cddb..eb878b933fe5ba4ee35a371d7149cd14ef161c2c 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -2062,8 +2062,7 @@ aarch64_init_data_intrinsics (void)
 void
 handle_arm_acle_h (void)
 {
-  if (TARGET_LS64)
-    aarch64_init_ls64_builtins ();
+  aarch64_init_ls64_builtins ();
   aarch64_init_tme_builtins ();
   aarch64_init_memtag_builtins ();
 }
@@ -2311,6 +2310,13 @@ aarch64_general_check_builtin_call (location_t location, vec<location_t>,
       return aarch64_check_required_extensions (location, decl,
 						AARCH64_FL_TME);
 
+    case AARCH64_LS64_BUILTIN_LD64B:
+    case AARCH64_LS64_BUILTIN_ST64B:
+    case AARCH64_LS64_BUILTIN_ST64BV:
+    case AARCH64_LS64_BUILTIN_ST64BV0:
+      return aarch64_check_required_extensions (location, decl,
+						AARCH64_FL_LS64);
+
     default:
       break;
     }
diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index ab04326791309796125860ce64e63fe858a4a733..ab4e7e60e046a9e9c81237de2ca5463c3d4f96ca 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -265,9 +265,7 @@ __crc32d (uint32_t __a, uint64_t __b)
 #define _TMFAILURE_INT        0x00800000u
 #define _TMFAILURE_TRIVIAL    0x01000000u
 
-#ifdef __ARM_FEATURE_LS64
 typedef __arm_data512_t data512_t;
-#endif
 
 #pragma GCC push_options
 #pragma GCC target ("+nothing+rng")
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-1.c b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..7dfc193a2934c994220280990316027c07e75ac4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.6-a" } */
+
+#include <arm_acle.h>
+
+data512_t foo (void * p)
+{
+  return __arm_ld64b (p); /* { dg-error {ACLE function '__arm_ld64b' requires ISA extension 'ls64'} } */
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-2.c b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..3ede05a81f026f8606ee2c9cd56f15ce45caa1c8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8.6-a" } */
+
+#include <arm_acle.h>
+
+#pragma GCC target("arch=armv8-a+ls64")
+data512_t foo (void * p)
+{
+  return __arm_ld64b (p);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-3.c b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..e0fccdad7bec4aa522fb709d010289fd02f91d05
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-3.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a+ls64 -mgeneral-regs-only" } */
+
+#include <arm_acle.h>
+
+data512_t foo (void * p)
+{
+  return __arm_ld64b (p);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-4.c b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-4.c
new file mode 100644
index 0000000000000000000000000000000000000000..af1d9a4241fd0047c52735a8103eeaa45525ffc0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/ls64_guard-4.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv8-a+ls64" } */
+
+#include <arm_acle.h>
+
+#pragma GCC target("arch=armv8.6-a")
+data512_t foo (void * p)
+{
+  return __arm_ld64b (p); /* { dg-error {ACLE function '__arm_ld64b' requires ISA extension 'ls64'} } */
+}

Re: [PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[Richard Sandiford]

Andrew Carlotti <andrew.carlotti@arm.com> writes:
> This series of patches fixes issues with some intrinsics being incorrectly
> gated by global target options, instad of just using function-specific target
> options.  These issues have been present since the +tme, +memtag and +ls64
> intrinsics were introduced.
>
> Compared to the previous version, this series no longer adds feature checks to
> the intrinsic expanders, and fixes various formatting issues pointed out by
> Richard Sandiford.
>
> Additionally, the series now refactors the checking of TARGET_GENERAL_REGS_ONLY
> in check_required_extensions.  This refactor is included as a new patch (1/5)
> to make the diffs more readable.
>
>
> Bootstrapped and regression tested on aarch64.  Ok to merge?

LGTM, thanks.  OK if there are no other comments before the weekend.

> Also, ok for backports to affected versions (with regression tests)?

Hmm, it seems a bit invasive.  And if the GCC 11 tag in the PR is
anything to go by, it sounds like this is already unfixable behaviour
in at least one release series.

Let's see if anyone else has any opinions.

Richard

Re: [PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[Andrew Carlotti]

On Thu, Aug 15, 2024 at 05:15:03PM +0100, Richard Sandiford wrote:
> Andrew Carlotti <andrew.carlotti@arm.com> writes:
> > This series of patches fixes issues with some intrinsics being incorrectly
> > gated by global target options, instad of just using function-specific target
> > options.  These issues have been present since the +tme, +memtag and +ls64
> > intrinsics were introduced.
> >
> > Compared to the previous version, this series no longer adds feature checks to
> > the intrinsic expanders, and fixes various formatting issues pointed out by
> > Richard Sandiford.
> >
> > Additionally, the series now refactors the checking of TARGET_GENERAL_REGS_ONLY
> > in check_required_extensions.  This refactor is included as a new patch (1/5)
> > to make the diffs more readable.
> >
> >
> > Bootstrapped and regression tested on aarch64.  Ok to merge?
> 
> LGTM, thanks.  OK if there are no other comments before the weekend.
> 
> > Also, ok for backports to affected versions (with regression tests)?
> 
> Hmm, it seems a bit invasive.  And if the GCC 11 tag in the PR is
> anything to go by, it sounds like this is already unfixable behaviour
> in at least one release series.

I think the impact is minimal prior to FMV support, so backporting is less
important for older versions.  The series should backport cleanly to GCC 14,
but would have conflicts in earlier version, so I think it would be sensible to
backport to GCC 14 and not further.

> Let's see if anyone else has any opinions.
> 
> Richard

Re: [PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[Kyrylo Tkachov]

> On 15 Aug 2024, at 18:48, Andrew Carlotti <andrew.carlotti@arm.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Aug 15, 2024 at 05:15:03PM +0100, Richard Sandiford wrote:
>> Andrew Carlotti <andrew.carlotti@arm.com> writes:
>>> This series of patches fixes issues with some intrinsics being incorrectly
>>> gated by global target options, instad of just using function-specific target
>>> options.  These issues have been present since the +tme, +memtag and +ls64
>>> intrinsics were introduced.
>>> 
>>> Compared to the previous version, this series no longer adds feature checks to
>>> the intrinsic expanders, and fixes various formatting issues pointed out by
>>> Richard Sandiford.
>>> 
>>> Additionally, the series now refactors the checking of TARGET_GENERAL_REGS_ONLY
>>> in check_required_extensions.  This refactor is included as a new patch (1/5)
>>> to make the diffs more readable.
>>> 
>>> 
>>> Bootstrapped and regression tested on aarch64.  Ok to merge?
>> 
>> LGTM, thanks.  OK if there are no other comments before the weekend.
>> 
>>> Also, ok for backports to affected versions (with regression tests)?
>> 
>> Hmm, it seems a bit invasive.  And if the GCC 11 tag in the PR is
>> anything to go by, it sounds like this is already unfixable behaviour
>> in at least one release series.
> 
> I think the impact is minimal prior to FMV support, so backporting is less
> important for older versions.  The series should backport cleanly to GCC 14,
> but would have conflicts in earlier version, so I think it would be sensible to
> backport to GCC 14 and not further.

I think backporting only to GCC 14 is sensible. The intrinsics in question tbh are or will be shipping hardware that I don’t expect will be used with older compilers much to be worth the risk of adjusting the patches for those branches.
Thanks,
Kyrill


> 
>> Let's see if anyone else has any opinions.
>> 
>> Richard

Re: [PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[Andrew Carlotti]

On Fri, Aug 16, 2024 at 07:17:24AM +0000, Kyrylo Tkachov wrote:
> 
> 
> > On 15 Aug 2024, at 18:48, Andrew Carlotti <andrew.carlotti@arm.com> wrote:
> > 
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Thu, Aug 15, 2024 at 05:15:03PM +0100, Richard Sandiford wrote:
> >> Andrew Carlotti <andrew.carlotti@arm.com> writes:
> >>> This series of patches fixes issues with some intrinsics being incorrectly
> >>> gated by global target options, instad of just using function-specific target
> >>> options.  These issues have been present since the +tme, +memtag and +ls64
> >>> intrinsics were introduced.
> >>> 
> >>> Compared to the previous version, this series no longer adds feature checks to
> >>> the intrinsic expanders, and fixes various formatting issues pointed out by
> >>> Richard Sandiford.
> >>> 
> >>> Additionally, the series now refactors the checking of TARGET_GENERAL_REGS_ONLY
> >>> in check_required_extensions.  This refactor is included as a new patch (1/5)
> >>> to make the diffs more readable.
> >>> 
> >>> 
> >>> Bootstrapped and regression tested on aarch64.  Ok to merge?
> >> 
> >> LGTM, thanks.  OK if there are no other comments before the weekend.
> >> 
> >>> Also, ok for backports to affected versions (with regression tests)?
> >> 
> >> Hmm, it seems a bit invasive.  And if the GCC 11 tag in the PR is
> >> anything to go by, it sounds like this is already unfixable behaviour
> >> in at least one release series.
> > 
> > I think the impact is minimal prior to FMV support, so backporting is less
> > important for older versions.  The series should backport cleanly to GCC 14,
> > but would have conflicts in earlier version, so I think it would be sensible to
> > backport to GCC 14 and not further.
> 
> I think backporting only to GCC 14 is sensible. The intrinsics in question tbh are or will be shipping hardware that I don’t expect will be used with older compilers much to be worth the risk of adjusting the patches for those branches.
> Thanks,
> Kyrill
> 
> 
> > 
> >> Let's see if anyone else has any opinions.
> >> 
> >> Richard
> 

I've pushed this to master now (with a couple of Changelog fixes).  I'll
backport it to GCC 14 next week if there are no issues.

Re: [PATCH v3 0/5] aarch64: Fix intrinsic availability [PR112108]

[Andrew Carlotti]

On Mon, Aug 19, 2024 at 03:52:58PM +0100, Andrew Carlotti wrote:
> On Fri, Aug 16, 2024 at 07:17:24AM +0000, Kyrylo Tkachov wrote:
> > 
> > 
> > > On 15 Aug 2024, at 18:48, Andrew Carlotti <andrew.carlotti@arm.com> wrote:
> > > 
> > > External email: Use caution opening links or attachments
> > > 
> > > 
> > > On Thu, Aug 15, 2024 at 05:15:03PM +0100, Richard Sandiford wrote:
> > >> Andrew Carlotti <andrew.carlotti@arm.com> writes:
> > >>> This series of patches fixes issues with some intrinsics being incorrectly
> > >>> gated by global target options, instad of just using function-specific target
> > >>> options.  These issues have been present since the +tme, +memtag and +ls64
> > >>> intrinsics were introduced.
> > >>> 
> > >>> Compared to the previous version, this series no longer adds feature checks to
> > >>> the intrinsic expanders, and fixes various formatting issues pointed out by
> > >>> Richard Sandiford.
> > >>> 
> > >>> Additionally, the series now refactors the checking of TARGET_GENERAL_REGS_ONLY
> > >>> in check_required_extensions.  This refactor is included as a new patch (1/5)
> > >>> to make the diffs more readable.
> > >>> 
> > >>> 
> > >>> Bootstrapped and regression tested on aarch64.  Ok to merge?
> > >> 
> > >> LGTM, thanks.  OK if there are no other comments before the weekend.
> > >> 
> > >>> Also, ok for backports to affected versions (with regression tests)?
> > >> 
> > >> Hmm, it seems a bit invasive.  And if the GCC 11 tag in the PR is
> > >> anything to go by, it sounds like this is already unfixable behaviour
> > >> in at least one release series.
> > > 
> > > I think the impact is minimal prior to FMV support, so backporting is less
> > > important for older versions.  The series should backport cleanly to GCC 14,
> > > but would have conflicts in earlier version, so I think it would be sensible to
> > > backport to GCC 14 and not further.
> > 
> > I think backporting only to GCC 14 is sensible. The intrinsics in question tbh are or will be shipping hardware that I don’t expect will be used with older compilers much to be worth the risk of adjusting the patches for those branches.
> > Thanks,
> > Kyrill
> > 
> > 
> > > 
> > >> Let's see if anyone else has any opinions.
> > >> 
> > >> Richard
> > 
> 
> I've pushed this to master now (with a couple of Changelog fixes).  I'll
> backport it to GCC 14 next week if there are no issues.

Backported cleanly to GCC 14, and pushed after passing regression testing.