This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64][11/14] Re-layout SIMD builtin types on builtin expansion
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Wed, 22 Jul 2015 10:11:38 +0100
- Subject: Re: [PATCH][AArch64][11/14] Re-layout SIMD builtin types on builtin expansion
- Authentication-results: sourceware.org; auth=none
- References: <55A7CBE7 dot 4010204 at arm dot com> <55AE7A7B dot 9030707 at arm dot com>
On Tue, Jul 21, 2015 at 05:59:39PM +0100, Kyrill Tkachov wrote:
> Sorry, here's the correct version, which uses initialized instead of inited in one of the variable names.
Some nits below.
>
> Kyrill
>
> 2015-07-21 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> * config/aarch64/aarch64.c (aarch64_option_valid_attribute_p):
> Initialize simd builtins if TARGET_SIMD.
> * config/aarch64/aarch64-builtins.c (aarch64_init_simd_builtins):
> Make sure that the builtins are initialized only once no matter how
> many times the function is called.
> (aarch64_init_builtins): Unconditionally initialize crc builtins.
> (aarch64_relayout_simd_param): New function.
> (aarch64_simd_expand_args): Use above during argument expansion.
> * config/aarch64/aarch64-c.c (aarch64_pragma_target_parse): Initialize
> simd builtins if TARGET_SIMD.
> * config/aarch64/aarch64-protos.h (aarch64_init_simd_builtins): New
> prototype.
> (aarch64_relayout_simd_types): Likewise.
>
> 2015-07-21 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> * gcc.target/aarch64/target-attr-crypto-ice-1.c: New test.
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
> index ec60955..ae0ea5b 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -684,11 +684,18 @@ aarch64_init_simd_builtin_scalar_types (void)
> "__builtin_aarch64_simd_udi");
> }
>
> -static void
> +static bool simd_builtins_initialized_p = false;
This should be in the "aarch64_" "namespace". simd_builtins_initialized_p
sounds generic enough that it might one day collide.
> +
> +void
> aarch64_init_simd_builtins (void)
> {
> unsigned int i, fcode = AARCH64_SIMD_PATTERN_START;
>
> + if (simd_builtins_initialized_p)
> + return;
> +
> + simd_builtins_initialized_p = true;
> +
> aarch64_init_simd_builtin_types ();
>
> /* Strong-typing hasn't been implemented for all AdvSIMD builtin intrinsics.
> @@ -851,8 +858,8 @@ aarch64_init_builtins (void)
>
> if (TARGET_SIMD)
> aarch64_init_simd_builtins ();
> - if (TARGET_CRC32)
> - aarch64_init_crc32_builtins ();
> +
> + aarch64_init_crc32_builtins ();
> }
>
> tree
> @@ -872,6 +879,31 @@ typedef enum
> SIMD_ARG_STOP
> } builtin_simd_arg;
>
> +/* Relayout the decl of a function arg. Keep the RTL component the same,
> + as varasm.c ICEs at varasm.c:1324. It doesn't like reinitializing the RTL
I think hard coding the line number is probably not helpful as the code
base evolves.
> + on PARM decls. Something like this needs to be done when compiling a
> + file without SIMD and then tagging a function with +simd and using SIMD
> + intrinsics in there. The types will have been laid out assuming no SIMD,
> + so we want to re-lay them out. */
> +
> +static void
> +aarch64_relayout_simd_param (tree arg)
> +{
> + tree argdecl = arg;
> + if (TREE_CODE (argdecl) == SSA_NAME)
> + argdecl = SSA_NAME_VAR (argdecl);
> +
> + if (argdecl
> + && (TREE_CODE (argdecl) == PARM_DECL
> + || TREE_CODE (argdecl) == VAR_DECL))
> + {
> + rtx rtl = NULL_RTX;
> + rtl = DECL_RTL_IF_SET (argdecl);
> + relayout_decl (argdecl);
> + SET_DECL_RTL (argdecl, rtl);
> + }
> +}
> +
> static rtx
> aarch64_simd_expand_args (rtx target, int icode, int have_retval,
> tree exp, builtin_simd_arg *args)
> @@ -900,6 +932,7 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval,
> {
> tree arg = CALL_EXPR_ARG (exp, opc - have_retval);
> enum machine_mode mode = insn_data[icode].operand[opc].mode;
> + aarch64_relayout_simd_param (arg);
> op[opc] = expand_normal (arg);
>
> switch (thisarg)
> diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
> index c3798a1..ecc9974 100644
> --- a/gcc/config/aarch64/aarch64-c.c
> +++ b/gcc/config/aarch64/aarch64-c.c
> @@ -179,6 +179,19 @@ aarch64_pragma_target_parse (tree args, tree pop_target)
>
> cpp_opts->warn_unused_macros = saved_warn_unused_macros;
>
> + /* Initialize SIMD builtins if we haven't already.
> + Set current_target_pragma to NULL for the duration so that
> + the builtin initialization code doesn't try to tag the functions
> + being built with the attributes specified by any current pragma, thus
> + going into an infinite recursion. */
> + if (TARGET_SIMD)
> + {
> + tree saved_current_target_pragma = current_target_pragma;
> + current_target_pragma = NULL;
> + aarch64_init_simd_builtins ();
> + current_target_pragma = saved_current_target_pragma;
> + }
> +
> return ret;
> }
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 0191f35..4fe437f 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -382,6 +382,8 @@ extern bool aarch64_madd_needs_nop (rtx_insn *);
> extern void aarch64_final_prescan_insn (rtx_insn *);
> extern void aarch64_reset_previous_fndecl (void);
> extern void aarch64_cpu_cpp_builtins (cpp_reader *);
> +extern void aarch64_init_simd_builtins (void);
> +extern void aarch64_relayout_simd_types (void);
> extern void aarch64_register_pragmas (void);
> extern bool
> aarch64_expand_vec_perm_const (rtx target, rtx op0, rtx op1, rtx sel);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index b697487..9128866 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -8418,6 +8418,18 @@ aarch64_option_valid_attribute_p (tree fndecl, tree, tree args, int)
> if (ret)
> {
> aarch64_override_options_internal (&global_options);
> + /* Initialize SIMD builtins if we haven't already.
> + Set current_target_pragma to NULL for the duration so that
> + the builtin initialization code doesn't try to tag the functions
> + being built with the attributes specified by any current pragma, thus
> + going into an infinite recursion. */
8 spaces should become a tab.
> + if (TARGET_SIMD)
> + {
Likewise.
> + tree saved_current_target_pragma = current_target_pragma;
> + current_target_pragma = NULL;
> + aarch64_init_simd_builtins ();
> + current_target_pragma = saved_current_target_pragma;
> + }
Likewise.
> new_target = build_target_option_node (&global_options);
> }
> else
Thanks,
James