Andrew Carlotti <andrew.carlo...@arm.com> writes:
> 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.
> (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.
Same comments about reindentation and expand checking as for 2/4.
Also, one very minor nit:
> diff --git a/gcc/config/aarch64/aarch64-builtins.cc
> b/gcc/config/aarch64/aarch64-builtins.cc
> index
> f7d31d8c4308b4a883f8ce7df5c3ee319abbbb9c..50667e555497b483aea6a64bb5809ddc62cedf83
> 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -1936,7 +1936,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_simulate_builtin ("__arm_mte_"#N, \
> T, AARCH64_MEMTAG_BUILTIN_##F); \
> aarch64_memtag_builtin_data[AARCH64_MEMTAG_BUILTIN_##F - \
> AARCH64_MEMTAG_BUILTIN_START - 1] = \
> @@ -1944,19 +1944,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);
> @@ -2069,6 +2069,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. */
> @@ -2161,9 +2162,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 ();
> }
> @@ -2323,7 +2321,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, false);
The return statement should be indented by 4 rather than 8 columns.
LGTM otherwise, but please give others 24 hours to comment.
Thanks,
Richard
> +
> return true;
> }
>
> @@ -3098,6 +3101,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
> 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'} } */
> +}
