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.tkac...@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.tkac...@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