Andrew Carlotti <andrew.carlo...@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): Move tme initialisation...
> (handle_arm_acle_h): ...to here, and remove feature check.
> (aarch64_general_check_builtin_call): Check tme intrinsics.
> (aarch64_expand_builtin_tme): Check feature availability.
> * 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
> d0fb8bc1d1fedb382cba1a1f09a9c3ce6757ee22..f7d31d8c4308b4a883f8ce7df5c3ee319abbbb9c
> 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -1791,19 +1791,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_simulate_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_simulate_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_simulate_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_simulate_builtin ("__tcancel",
> ftype_void_uint64,
> AARCH64_TME_BUILTIN_TCANCEL);
Very minor, sorry, but could you reindent the arguments to match
the new function name?
> }
> @@ -2068,6 +2068,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 +2161,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 ();
>
> @@ -2289,6 +2287,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:
> @@ -2301,15 +2300,28 @@ 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, false);
> +
> + default:
> + break;
> }
> /* Default behavior. */
> return true;
> @@ -2734,6 +2746,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;
> +
Does this ever trigger? The SVE code also checks during expansion,
but I think that's because of the manual overload resolution. Perhaps
I misremember though...
LGTM otherwise, but please leave 24 hours for others to comment.
Thanks,
Richard
> switch (fcode)
> {
> case AARCH64_TME_BUILTIN_TSTART:
> 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'} } */
> +}
