public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [0/4] aarch64: Fix intrinsic availability [PR112108]
@ 2023-11-09 11:24 Andrew Carlotti
  2023-11-09 11:25 ` [1/4] aarch64: Refactor check_required_extensions Andrew Carlotti
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Andrew Carlotti @ 2023-11-09 11:24 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.earnshaw, richard.sandiford, kyrylo.tkachov

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

Bootstrapped and regression tested on aarch64.  Ok to merge?

Also, ok for backports to all open affected versions (with regression tests)?
I believe the first three patches will apply cleanly back to GCC 11.

The ls64 intrinsics were only added in GCC 12, and have recently had their
implementation changed, so I'll send a separate backport patch for approval
once this series is merged.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [1/4] aarch64: Refactor check_required_extensions
  2023-11-09 11:24 [0/4] aarch64: Fix intrinsic availability [PR112108] Andrew Carlotti
@ 2023-11-09 11:25 ` Andrew Carlotti
  2023-11-09 11:41   ` Richard Sandiford
  2023-11-09 11:26 ` [2/4] aarch64: Fix tme intrinsic availability Andrew Carlotti
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Andrew Carlotti @ 2023-11-09 11:25 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.earnshaw, richard.sandiford, kyrylo.tkachov

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, check_required_registers)
	(check_required_extensions): Move out of aarch64_sve namespace,
	rename, and move into...
	* config/aarch64/aarch64-builtins.cc (aarch64_report_missing_extension)
	(aarch64_check_non_general_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 04f59fd9a54306d6422b03e32dce79bc00aed4f8..11a9ba2256f105d8cb9cdc4d6decb5b2be3d69af 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -2054,6 +2054,90 @@ 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;
+}
+
+/* Check whether non-general registers required by ACLE function fndecl are
+ * available.  Report an error against LOCATION and return false if not.  */
+static bool
+aarch64_check_non_general_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)
+    {
+      /* FP/SIMD/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 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.
+   If REQUIRES_NON_GENERAL_REGISTERS is true, then also check whether
+   non-general registers are available.  */
+bool
+aarch64_check_required_extensions (location_t location, tree fndecl,
+			   aarch64_feature_flags required_extensions,
+			   bool requires_non_general_registers)
+{
+  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
+  if (missing_extensions == 0)
+    return requires_non_general_registers
+	   ? aarch64_check_non_general_registers (location, fndecl)
+	   : true;
+
+  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 ();
+}
+
+
 typedef enum
 {
   SIMD_ARG_COPY_TO_REG,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 60a55f4bc1956786ea687fc7cad7ec9e4a84e1f0..30726140a6945dcb86b787f8f47952810d99379f 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -988,6 +988,9 @@ 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 = true);
+
 namespace aarch64_sve {
   void init_builtins ();
   void handle_arm_sve_h ();
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index 161a14edde7c9fb1b13b146cf50463e2d78db264..c72baed08759e143e6b9bc4fabf115a8374af7c8 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -558,14 +558,6 @@ static GTY(()) vec<registered_function *, va_gc> *registered_functions;
    overloaded functions.  */
 static hash_table<registered_function_hasher> *function_table;
 
-/* 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.  */
@@ -648,75 +640,6 @@ find_type_suffix_for_scalar_type (const_tree type)
   return NUM_TYPE_SUFFIXES;
 }
 
-/* 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;
-}
-
-/* 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)
-{
-  /* 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 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)
-{
-  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
-  if (missing_extensions == 0)
-    return check_required_registers (location, fndecl);
-
-  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.  */
@@ -3605,7 +3528,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 ();
@@ -3628,8 +3552,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 ();
 }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [2/4] aarch64: Fix tme intrinsic availability
  2023-11-09 11:24 [0/4] aarch64: Fix intrinsic availability [PR112108] Andrew Carlotti
  2023-11-09 11:25 ` [1/4] aarch64: Refactor check_required_extensions Andrew Carlotti
@ 2023-11-09 11:26 ` Andrew Carlotti
  2023-11-10 10:34   ` Richard Sandiford
  2023-11-09 11:26 ` [3/4] aarch64: Fix memtag " Andrew Carlotti
  2023-11-09 11:27 ` [4/4] aarch64: Fix ls64 " Andrew Carlotti
  3 siblings, 1 reply; 8+ messages in thread
From: Andrew Carlotti @ 2023-11-09 11:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.earnshaw, richard.sandiford, kyrylo.tkachov

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): Remove feature check.
	(aarch64_check_general_builtin_call): New.
	(aarch64_expand_builtin_tme): Check feature availability.
	* config/aarch64/aarch64-c.cc (aarch64_check_builtin_call): Add
	check for non-SVE builtins.
	* config/aarch64/aarch64-protos.h (aarch64_check_general_builtin_call):
	New prototype.
	* 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 11a9ba2256f105d8cb9cdc4d6decb5b2be3d69af..ac0259a892e16adb5b241032ac3df1e7ab5370ef 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -1765,19 +1765,19 @@ 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",
+    = aarch64_general_add_builtin ("__tstart",
 				   ftype_uint64_void,
 				   AARCH64_TME_BUILTIN_TSTART);
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TTEST]
-    = aarch64_general_add_builtin ("__builtin_aarch64_ttest",
+    = aarch64_general_add_builtin ("__ttest",
 				   ftype_uint64_void,
 				   AARCH64_TME_BUILTIN_TTEST);
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCOMMIT]
-    = aarch64_general_add_builtin ("__builtin_aarch64_tcommit",
+    = aarch64_general_add_builtin ("__tcommit",
 				   ftype_void_void,
 				   AARCH64_TME_BUILTIN_TCOMMIT);
   aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCANCEL]
-    = aarch64_general_add_builtin ("__builtin_aarch64_tcancel",
+    = aarch64_general_add_builtin ("__tcancel",
 				   ftype_void_uint64,
 				   AARCH64_TME_BUILTIN_TCANCEL);
 }
@@ -2034,8 +2034,7 @@ aarch64_general_init_builtins (void)
   if (!TARGET_ILP32)
     aarch64_init_pauth_hint_builtins ();
 
-  if (TARGET_TME)
-    aarch64_init_tme_builtins ();
+  aarch64_init_tme_builtins ();
 
   if (TARGET_MEMTAG)
     aarch64_init_memtag_builtins ();
@@ -2137,6 +2136,24 @@ aarch64_check_required_extensions (location_t location, tree fndecl,
   gcc_unreachable ();
 }
 
+bool aarch64_check_general_builtin_call (location_t location,
+					 unsigned int fcode)
+{
+  tree fndecl = aarch64_builtin_decls[fcode];
+  switch (fcode)
+    {
+      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, fndecl,
+						  AARCH64_FL_TME, false);
+
+      default:
+	break;
+    }
+  return true;
+}
 
 typedef enum
 {
@@ -2559,6 +2576,11 @@ aarch64_expand_fcmla_builtin (tree exp, rtx target, int fcode)
 static rtx
 aarch64_expand_builtin_tme (int fcode, tree exp, rtx target)
 {
+  tree fndecl = aarch64_builtin_decls[fcode];
+  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), fndecl,
+					  AARCH64_FL_TME, false))
+    return target;
+
   switch (fcode)
     {
     case AARCH64_TME_BUILTIN_TSTART:
diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
index ab8844f6049dc95b97648b651bfcd3a4ccd3ca0b..6b6bd77e9e66cd2d9a211387e07d3e20d935fb1a 100644
--- a/gcc/config/aarch64/aarch64-c.cc
+++ b/gcc/config/aarch64/aarch64-c.cc
@@ -339,7 +339,7 @@ aarch64_check_builtin_call (location_t loc, vec<location_t> arg_loc,
   switch (code & AARCH64_BUILTIN_CLASS)
     {
     case AARCH64_BUILTIN_GENERAL:
-      return true;
+      return aarch64_check_general_builtin_call (loc, subcode);
 
     case AARCH64_BUILTIN_SVE:
       return aarch64_sve::check_builtin_call (loc, arg_loc, subcode,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 30726140a6945dcb86b787f8f47952810d99379f..94022f77d7e7bab2533d78965bec241b4070c729 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -991,6 +991,8 @@ void handle_arm_neon_h (void);
 bool aarch64_check_required_extensions (location_t, tree,
 					aarch64_feature_flags, bool = true);
 
+bool aarch64_check_general_builtin_call (location_t, unsigned int);
+
 namespace aarch64_sve {
   void init_builtins ();
   void handle_arm_sve_h ();
diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index 7599a32301dadf80760d3cb40a8685d2e6a476fb..f4e35d1e12ac9bbcc4f1b75d8e5baad62f8634a0 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -222,10 +222,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
@@ -238,37 +235,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'} } */
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [3/4] aarch64: Fix memtag intrinsic availability
  2023-11-09 11:24 [0/4] aarch64: Fix intrinsic availability [PR112108] Andrew Carlotti
  2023-11-09 11:25 ` [1/4] aarch64: Refactor check_required_extensions Andrew Carlotti
  2023-11-09 11:26 ` [2/4] aarch64: Fix tme intrinsic availability Andrew Carlotti
@ 2023-11-09 11:26 ` Andrew Carlotti
  2023-11-09 11:27 ` [4/4] aarch64: Fix ls64 " Andrew Carlotti
  3 siblings, 0 replies; 8+ messages in thread
From: Andrew Carlotti @ 2023-11-09 11:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.earnshaw, richard.sandiford, kyrylo.tkachov

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): Remove feature check.
	(aarch64_check_general_builtin_call): Check memtag intrinsics.
	(aarch64_expand_builtin_memtag): Add feature check.
	* 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 ac0259a892e16adb5b241032ac3df1e7ab5370ef..503d8ad98d7de959d8c7c78cef575d29e2132f78 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -1813,7 +1813,7 @@ 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, \
+    = aarch64_general_add_builtin ("__arm_mte_"#N, \
 				   T, AARCH64_MEMTAG_BUILTIN_##F); \
   aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
 			      AARCH64_MEMTAG_BUILTIN_START - 1] = \
@@ -1821,19 +1821,19 @@ aarch64_init_memtag_builtins (void)
 
   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);
@@ -2036,8 +2036,7 @@ aarch64_general_init_builtins (void)
 
   aarch64_init_tme_builtins ();
 
-  if (TARGET_MEMTAG)
-    aarch64_init_memtag_builtins ();
+  aarch64_init_memtag_builtins ();
 
   if (in_lto_p)
     handle_arm_acle_h ();
@@ -2152,6 +2151,12 @@ bool aarch64_check_general_builtin_call (location_t location,
       default:
 	break;
     }
+
+  if (fcode >= AARCH64_MEMTAG_BUILTIN_START
+      && fcode <= AARCH64_MEMTAG_BUILTIN_END)
+	return aarch64_check_required_extensions (location, fndecl,
+						  AARCH64_FL_MEMTAG, false);
+
   return true;
 }
 
@@ -2716,6 +2721,11 @@ aarch64_expand_builtin_memtag (int fcode, tree exp, rtx target)
       return const0_rtx;
     }
 
+  tree fndecl = aarch64_builtin_decls[fcode];
+  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), fndecl,
+					  AARCH64_FL_MEMTAG, false))
+    return target;
+
   rtx pat = NULL;
   enum insn_code icode = aarch64_memtag_builtin_data[fcode -
 			   AARCH64_MEMTAG_BUILTIN_START - 1].icode;
diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index f4e35d1e12ac9bbcc4f1b75d8e5baad62f8634a0..57f16603d22cec81002b00b94afe1201c83b4b94 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -257,29 +257,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
-
 #ifdef __cplusplus
 }
 #endif
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'} } */
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [4/4] aarch64: Fix ls64 intrinsic availability
  2023-11-09 11:24 [0/4] aarch64: Fix intrinsic availability [PR112108] Andrew Carlotti
                   ` (2 preceding siblings ...)
  2023-11-09 11:26 ` [3/4] aarch64: Fix memtag " Andrew Carlotti
@ 2023-11-09 11:27 ` Andrew Carlotti
  3 siblings, 0 replies; 8+ messages in thread
From: Andrew Carlotti @ 2023-11-09 11:27 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.earnshaw, richard.sandiford, kyrylo.tkachov

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_check_general_builtin_call): Check ls64 intrinsics.
	(aarch64_expand_builtin_ls64): Add feature check.
	* 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 503d8ad98d7de959d8c7c78cef575d29e2132f78..9fd0d5c362815c25793bc04a1d82e32bd30bbc22 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -1943,8 +1943,7 @@ aarch64_init_data_intrinsics (void)
 void
 handle_arm_acle_h (void)
 {
-  if (TARGET_LS64)
-    aarch64_init_ls64_builtins ();
+  aarch64_init_ls64_builtins ();
 }
 
 /* Initialize fpsr fpcr getters and setters.  */
@@ -2148,6 +2147,13 @@ bool aarch64_check_general_builtin_call (location_t location,
 	return aarch64_check_required_extensions (location, fndecl,
 						  AARCH64_FL_TME, false);
 
+      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, fndecl,
+						  AARCH64_FL_LS64, false);
+
       default:
 	break;
     }
@@ -2630,6 +2636,11 @@ aarch64_expand_builtin_ls64 (int fcode, tree exp, rtx target)
 {
   expand_operand ops[3];
 
+  tree fndecl = aarch64_builtin_decls[fcode];
+  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), fndecl,
+					  AARCH64_FL_LS64, false))
+    return target;
+
   switch (fcode)
     {
     case AARCH64_LS64_BUILTIN_LD64B:
diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index 57f16603d22cec81002b00b94afe1201c83b4b94..e7aae7e5278691508086e6438b57b8a6fb6df554 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -235,9 +235,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'} } */
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [1/4] aarch64: Refactor check_required_extensions
  2023-11-09 11:25 ` [1/4] aarch64: Refactor check_required_extensions Andrew Carlotti
@ 2023-11-09 11:41   ` Richard Sandiford
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2023-11-09 11:41 UTC (permalink / raw)
  To: Andrew Carlotti; +Cc: gcc-patches, richard.earnshaw, kyrylo.tkachov

Andrew Carlotti <andrew.carlotti@arm.com> writes:
> 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, check_required_registers)
> 	(check_required_extensions): Move out of aarch64_sve namespace,
> 	rename, and move into...
> 	* config/aarch64/aarch64-builtins.cc (aarch64_report_missing_extension)
> 	(aarch64_check_non_general_registers)
> 	(aarch64_check_required_extensions) ...here.
> 	* config/aarch64/aarch64-protos.h (aarch64_check_required_extensions):
> 	Add prototype.

OK for trunk and backports, but...

> diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
> index 04f59fd9a54306d6422b03e32dce79bc00aed4f8..11a9ba2256f105d8cb9cdc4d6decb5b2be3d69af 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -2054,6 +2054,90 @@ 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)

...this needs reindenting for the longer name.  Same for
aarch64_check_required_extensions.

Thanks,
Richard


> +{
> +  /* 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;
> +}
> +
> +/* Check whether non-general registers required by ACLE function fndecl are
> + * available.  Report an error against LOCATION and return false if not.  */
> +static bool
> +aarch64_check_non_general_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)
> +    {
> +      /* FP/SIMD/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 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.
> +   If REQUIRES_NON_GENERAL_REGISTERS is true, then also check whether
> +   non-general registers are available.  */
> +bool
> +aarch64_check_required_extensions (location_t location, tree fndecl,
> +			   aarch64_feature_flags required_extensions,
> +			   bool requires_non_general_registers)
> +{
> +  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
> +  if (missing_extensions == 0)
> +    return requires_non_general_registers
> +	   ? aarch64_check_non_general_registers (location, fndecl)
> +	   : true;
> +
> +  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 ();
> +}
> +
> +
>  typedef enum
>  {
>    SIMD_ARG_COPY_TO_REG,
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 60a55f4bc1956786ea687fc7cad7ec9e4a84e1f0..30726140a6945dcb86b787f8f47952810d99379f 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -988,6 +988,9 @@ 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 = true);
> +
>  namespace aarch64_sve {
>    void init_builtins ();
>    void handle_arm_sve_h ();
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index 161a14edde7c9fb1b13b146cf50463e2d78db264..c72baed08759e143e6b9bc4fabf115a8374af7c8 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -558,14 +558,6 @@ static GTY(()) vec<registered_function *, va_gc> *registered_functions;
>     overloaded functions.  */
>  static hash_table<registered_function_hasher> *function_table;
>  
> -/* 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.  */
> @@ -648,75 +640,6 @@ find_type_suffix_for_scalar_type (const_tree type)
>    return NUM_TYPE_SUFFIXES;
>  }
>  
> -/* 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;
> -}
> -
> -/* 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)
> -{
> -  /* 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 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)
> -{
> -  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
> -  if (missing_extensions == 0)
> -    return check_required_registers (location, fndecl);
> -
> -  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.  */
> @@ -3605,7 +3528,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 ();
> @@ -3628,8 +3552,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 ();
>  }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [2/4] aarch64: Fix tme intrinsic availability
  2023-11-09 11:26 ` [2/4] aarch64: Fix tme intrinsic availability Andrew Carlotti
@ 2023-11-10 10:34   ` Richard Sandiford
  2023-11-10 12:17     ` Andrew Carlotti
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2023-11-10 10:34 UTC (permalink / raw)
  To: Andrew Carlotti; +Cc: gcc-patches, richard.earnshaw, kyrylo.tkachov

Andrew Carlotti <andrew.carlotti@arm.com> writes:
> 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): Remove feature check.
> 	(aarch64_check_general_builtin_call): New.
> 	(aarch64_expand_builtin_tme): Check feature availability.
> 	* config/aarch64/aarch64-c.cc (aarch64_check_builtin_call): Add
> 	check for non-SVE builtins.
> 	* config/aarch64/aarch64-protos.h (aarch64_check_general_builtin_call):
> 	New prototype.
> 	* config/aarch64/arm_acle.h (__tstart, __tcommit, __tcancel)
> 	(__ttest): Remove.
> 	(_TMFAILURE_*): Define unconditionally.

My main concern with this is that it makes the functions available
even without including the header file.  That's fine from a namespace
pollution PoV, since the functions are in the implementation namespace.
But it might reduce code portability if GCC allows these ACLE functions
to be used without including the header file, while other compilers
require the header file.

For LS64 we instead used a pragma to trigger the definition of the
functions (requiring aarch64_general_simulate_builtin rather than
aarch64_general_add_builtin).  I think it'd be better to do the same here.

> 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 11a9ba2256f105d8cb9cdc4d6decb5b2be3d69af..ac0259a892e16adb5b241032ac3df1e7ab5370ef 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -1765,19 +1765,19 @@ 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",
> +    = aarch64_general_add_builtin ("__tstart",
>  				   ftype_uint64_void,
>  				   AARCH64_TME_BUILTIN_TSTART);
>    aarch64_builtin_decls[AARCH64_TME_BUILTIN_TTEST]
> -    = aarch64_general_add_builtin ("__builtin_aarch64_ttest",
> +    = aarch64_general_add_builtin ("__ttest",
>  				   ftype_uint64_void,
>  				   AARCH64_TME_BUILTIN_TTEST);
>    aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCOMMIT]
> -    = aarch64_general_add_builtin ("__builtin_aarch64_tcommit",
> +    = aarch64_general_add_builtin ("__tcommit",
>  				   ftype_void_void,
>  				   AARCH64_TME_BUILTIN_TCOMMIT);
>    aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCANCEL]
> -    = aarch64_general_add_builtin ("__builtin_aarch64_tcancel",
> +    = aarch64_general_add_builtin ("__tcancel",
>  				   ftype_void_uint64,
>  				   AARCH64_TME_BUILTIN_TCANCEL);
>  }
> @@ -2034,8 +2034,7 @@ aarch64_general_init_builtins (void)
>    if (!TARGET_ILP32)
>      aarch64_init_pauth_hint_builtins ();
>  
> -  if (TARGET_TME)
> -    aarch64_init_tme_builtins ();
> +  aarch64_init_tme_builtins ();
>  
>    if (TARGET_MEMTAG)
>      aarch64_init_memtag_builtins ();
> @@ -2137,6 +2136,24 @@ aarch64_check_required_extensions (location_t location, tree fndecl,
>    gcc_unreachable ();
>  }
>  
> +bool aarch64_check_general_builtin_call (location_t location,
> +					 unsigned int fcode)

Formatting trivia: should be a line break after the "bool".  Would be
worth having a comment like:

/* Implement TARGET_CHECK_BUILTIN_CALL for the AARCH64_BUILTIN_GENERAL
   group.  */

"aarch64_general_check_builtin_call" would avoid splitting the name
of the target hook.

Thanks,
Richard

> +{
> +  tree fndecl = aarch64_builtin_decls[fcode];
> +  switch (fcode)
> +    {
> +      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, fndecl,
> +						  AARCH64_FL_TME, false);
> +
> +      default:
> +	break;
> +    }
> +  return true;
> +}
>  
>  typedef enum
>  {
> @@ -2559,6 +2576,11 @@ aarch64_expand_fcmla_builtin (tree exp, rtx target, int fcode)
>  static rtx
>  aarch64_expand_builtin_tme (int fcode, tree exp, rtx target)
>  {
> +  tree fndecl = aarch64_builtin_decls[fcode];
> +  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), fndecl,
> +					  AARCH64_FL_TME, false))
> +    return target;
> +
>    switch (fcode)
>      {
>      case AARCH64_TME_BUILTIN_TSTART:
> diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
> index ab8844f6049dc95b97648b651bfcd3a4ccd3ca0b..6b6bd77e9e66cd2d9a211387e07d3e20d935fb1a 100644
> --- a/gcc/config/aarch64/aarch64-c.cc
> +++ b/gcc/config/aarch64/aarch64-c.cc
> @@ -339,7 +339,7 @@ aarch64_check_builtin_call (location_t loc, vec<location_t> arg_loc,
>    switch (code & AARCH64_BUILTIN_CLASS)
>      {
>      case AARCH64_BUILTIN_GENERAL:
> -      return true;
> +      return aarch64_check_general_builtin_call (loc, subcode);
>  
>      case AARCH64_BUILTIN_SVE:
>        return aarch64_sve::check_builtin_call (loc, arg_loc, subcode,
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 30726140a6945dcb86b787f8f47952810d99379f..94022f77d7e7bab2533d78965bec241b4070c729 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -991,6 +991,8 @@ void handle_arm_neon_h (void);
>  bool aarch64_check_required_extensions (location_t, tree,
>  					aarch64_feature_flags, bool = true);
>  
> +bool aarch64_check_general_builtin_call (location_t, unsigned int);
> +
>  namespace aarch64_sve {
>    void init_builtins ();
>    void handle_arm_sve_h ();
> diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
> index 7599a32301dadf80760d3cb40a8685d2e6a476fb..f4e35d1e12ac9bbcc4f1b75d8e5baad62f8634a0 100644
> --- a/gcc/config/aarch64/arm_acle.h
> +++ b/gcc/config/aarch64/arm_acle.h
> @@ -222,10 +222,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
> @@ -238,37 +235,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'} } */
> +}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [2/4] aarch64: Fix tme intrinsic availability
  2023-11-10 10:34   ` Richard Sandiford
@ 2023-11-10 12:17     ` Andrew Carlotti
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Carlotti @ 2023-11-10 12:17 UTC (permalink / raw)
  To: gcc-patches, richard.earnshaw, kyrylo.tkachov, richard.sandiford

On Fri, Nov 10, 2023 at 10:34:29AM +0000, Richard Sandiford wrote:
> Andrew Carlotti <andrew.carlotti@arm.com> writes:
> > 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): Remove feature check.
> > 	(aarch64_check_general_builtin_call): New.
> > 	(aarch64_expand_builtin_tme): Check feature availability.
> > 	* config/aarch64/aarch64-c.cc (aarch64_check_builtin_call): Add
> > 	check for non-SVE builtins.
> > 	* config/aarch64/aarch64-protos.h (aarch64_check_general_builtin_call):
> > 	New prototype.
> > 	* config/aarch64/arm_acle.h (__tstart, __tcommit, __tcancel)
> > 	(__ttest): Remove.
> > 	(_TMFAILURE_*): Define unconditionally.
> 
> My main concern with this is that it makes the functions available
> even without including the header file.  That's fine from a namespace
> pollution PoV, since the functions are in the implementation namespace.
> But it might reduce code portability if GCC allows these ACLE functions
> to be used without including the header file, while other compilers
> require the header file.
> 
> For LS64 we instead used a pragma to trigger the definition of the
> functions (requiring aarch64_general_simulate_builtin rather than
> aarch64_general_add_builtin).  I think it'd be better to do the same here.

Good point - this is also the same as some simd intrinsic stuff I changed last
year.  I'll fix this in an updated patch, which will then also need a slightly
different version for backporting.

> > 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 11a9ba2256f105d8cb9cdc4d6decb5b2be3d69af..ac0259a892e16adb5b241032ac3df1e7ab5370ef 100644
> > --- a/gcc/config/aarch64/aarch64-builtins.cc
> > +++ b/gcc/config/aarch64/aarch64-builtins.cc
> > @@ -1765,19 +1765,19 @@ 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",
> > +    = aarch64_general_add_builtin ("__tstart",
> >  				   ftype_uint64_void,
> >  				   AARCH64_TME_BUILTIN_TSTART);
> >    aarch64_builtin_decls[AARCH64_TME_BUILTIN_TTEST]
> > -    = aarch64_general_add_builtin ("__builtin_aarch64_ttest",
> > +    = aarch64_general_add_builtin ("__ttest",
> >  				   ftype_uint64_void,
> >  				   AARCH64_TME_BUILTIN_TTEST);
> >    aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCOMMIT]
> > -    = aarch64_general_add_builtin ("__builtin_aarch64_tcommit",
> > +    = aarch64_general_add_builtin ("__tcommit",
> >  				   ftype_void_void,
> >  				   AARCH64_TME_BUILTIN_TCOMMIT);
> >    aarch64_builtin_decls[AARCH64_TME_BUILTIN_TCANCEL]
> > -    = aarch64_general_add_builtin ("__builtin_aarch64_tcancel",
> > +    = aarch64_general_add_builtin ("__tcancel",
> >  				   ftype_void_uint64,
> >  				   AARCH64_TME_BUILTIN_TCANCEL);
> >  }
> > @@ -2034,8 +2034,7 @@ aarch64_general_init_builtins (void)
> >    if (!TARGET_ILP32)
> >      aarch64_init_pauth_hint_builtins ();
> >  
> > -  if (TARGET_TME)
> > -    aarch64_init_tme_builtins ();
> > +  aarch64_init_tme_builtins ();
> >  
> >    if (TARGET_MEMTAG)
> >      aarch64_init_memtag_builtins ();
> > @@ -2137,6 +2136,24 @@ aarch64_check_required_extensions (location_t location, tree fndecl,
> >    gcc_unreachable ();
> >  }
> >  
> > +bool aarch64_check_general_builtin_call (location_t location,
> > +					 unsigned int fcode)
> 
> Formatting trivia: should be a line break after the "bool".  Would be
> worth having a comment like:
> 
> /* Implement TARGET_CHECK_BUILTIN_CALL for the AARCH64_BUILTIN_GENERAL
>    group.  */
> 
> "aarch64_general_check_builtin_call" would avoid splitting the name
> of the target hook.
> 
> Thanks,
> Richard
> 
> > +{
> > +  tree fndecl = aarch64_builtin_decls[fcode];
> > +  switch (fcode)
> > +    {
> > +      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, fndecl,
> > +						  AARCH64_FL_TME, false);
> > +
> > +      default:
> > +	break;
> > +    }
> > +  return true;
> > +}
> >  
> >  typedef enum
> >  {
> > @@ -2559,6 +2576,11 @@ aarch64_expand_fcmla_builtin (tree exp, rtx target, int fcode)
> >  static rtx
> >  aarch64_expand_builtin_tme (int fcode, tree exp, rtx target)
> >  {
> > +  tree fndecl = aarch64_builtin_decls[fcode];
> > +  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), fndecl,
> > +					  AARCH64_FL_TME, false))
> > +    return target;
> > +
> >    switch (fcode)
> >      {
> >      case AARCH64_TME_BUILTIN_TSTART:
> > diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
> > index ab8844f6049dc95b97648b651bfcd3a4ccd3ca0b..6b6bd77e9e66cd2d9a211387e07d3e20d935fb1a 100644
> > --- a/gcc/config/aarch64/aarch64-c.cc
> > +++ b/gcc/config/aarch64/aarch64-c.cc
> > @@ -339,7 +339,7 @@ aarch64_check_builtin_call (location_t loc, vec<location_t> arg_loc,
> >    switch (code & AARCH64_BUILTIN_CLASS)
> >      {
> >      case AARCH64_BUILTIN_GENERAL:
> > -      return true;
> > +      return aarch64_check_general_builtin_call (loc, subcode);
> >  
> >      case AARCH64_BUILTIN_SVE:
> >        return aarch64_sve::check_builtin_call (loc, arg_loc, subcode,
> > diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> > index 30726140a6945dcb86b787f8f47952810d99379f..94022f77d7e7bab2533d78965bec241b4070c729 100644
> > --- a/gcc/config/aarch64/aarch64-protos.h
> > +++ b/gcc/config/aarch64/aarch64-protos.h
> > @@ -991,6 +991,8 @@ void handle_arm_neon_h (void);
> >  bool aarch64_check_required_extensions (location_t, tree,
> >  					aarch64_feature_flags, bool = true);
> >  
> > +bool aarch64_check_general_builtin_call (location_t, unsigned int);
> > +
> >  namespace aarch64_sve {
> >    void init_builtins ();
> >    void handle_arm_sve_h ();
> > diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
> > index 7599a32301dadf80760d3cb40a8685d2e6a476fb..f4e35d1e12ac9bbcc4f1b75d8e5baad62f8634a0 100644
> > --- a/gcc/config/aarch64/arm_acle.h
> > +++ b/gcc/config/aarch64/arm_acle.h
> > @@ -222,10 +222,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
> > @@ -238,37 +235,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'} } */
> > +}

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-11-10 12:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-09 11:24 [0/4] aarch64: Fix intrinsic availability [PR112108] Andrew Carlotti
2023-11-09 11:25 ` [1/4] aarch64: Refactor check_required_extensions Andrew Carlotti
2023-11-09 11:41   ` Richard Sandiford
2023-11-09 11:26 ` [2/4] aarch64: Fix tme intrinsic availability Andrew Carlotti
2023-11-10 10:34   ` Richard Sandiford
2023-11-10 12:17     ` Andrew Carlotti
2023-11-09 11:26 ` [3/4] aarch64: Fix memtag " Andrew Carlotti
2023-11-09 11:27 ` [4/4] aarch64: Fix ls64 " Andrew Carlotti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).