Message ID | 55D4878F.4040803@arm.com |
---|---|
State | New |
Headers | show |
Ping. https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01084.html Thanks, Kyrill On 19/08/15 14:41, Kyrill Tkachov wrote: > Hi all, > > This fixes the ICE exposed by Alexandre's patch (https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00873.html) > The solution I came up with is to re-layout the parameter decls not during expansion time (when RTL has already > been allocated to SSA names) but in TARGET_SET_CURRENT_FUNCTION which is called much earlier before that and is > used when setting cfun. This way we reach expand with the proper vector modes registered for the param decls > and all seems to work ok. > > The aarch64-builtins.c workaround that I initially introduced in https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02012.html > are partially reverted (at least the re-laying out parts). > > The patch fixes the target_attr_crypto_ice_1.c ICE but I'd like to add a second derived testcase that > tests a different expansion path and it has proved useful in writing this patch. > > Bootstrapped and tested on aarch64. > > Ok for trunk? > > Thanks, > Kyrill > > 2015-08-19 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * config/aarch64/aarch64.c (aarch64_set_current_function): > Re-layout any vector parameters have non-simd layout. > * config/aarch64/aarch64-builtins.c (aarch64_relayout_simd_param): > Delete. > (aarch64_simd_expand_args): Delete call to the above. > > 2015-08-19 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * gcc.target/aarch64/target_attr_crypto_ice_2.c: New test.
On Wed, Aug 19, 2015 at 02:41:35PM +0100, Kyrill Tkachov wrote: > Hi all, > > This fixes the ICE exposed by Alexandre's patch (https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00873.html) > The solution I came up with is to re-layout the parameter decls not during expansion time (when RTL has already > been allocated to SSA names) but in TARGET_SET_CURRENT_FUNCTION which is called much earlier before that and is > used when setting cfun. This way we reach expand with the proper vector modes registered for the param decls > and all seems to work ok. > > The aarch64-builtins.c workaround that I initially introduced in https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02012.html > are partially reverted (at least the re-laying out parts). > > The patch fixes the target_attr_crypto_ice_1.c ICE but I'd like to add a second derived testcase that > tests a different expansion path and it has proved useful in writing this patch. > > Bootstrapped and tested on aarch64. > > Ok for trunk? OK. Thanks, James > 2015-08-19 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * config/aarch64/aarch64.c (aarch64_set_current_function): > Re-layout any vector parameters have non-simd layout. > * config/aarch64/aarch64-builtins.c (aarch64_relayout_simd_param): > Delete. > (aarch64_simd_expand_args): Delete call to the above. > > 2015-08-19 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * gcc.target/aarch64/target_attr_crypto_ice_2.c: New test.
commit 94093e43f5bc91f3afa1002f41dcd423e8db3237 Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com> Date: Tue Aug 18 17:02:26 2015 +0100 [AArch64] Re-layout vector parameter DECLs in TARGET_SET_CURRENT_FUNCTION diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c index 0f4f2b9..e3a90b5 100644 --- a/gcc/config/aarch64/aarch64-builtins.c +++ b/gcc/config/aarch64/aarch64-builtins.c @@ -886,30 +886,6 @@ 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. It doesn't like reinitializing the RTL - 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, @@ -940,7 +916,6 @@ 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 b/gcc/config/aarch64/aarch64.c index 217b4d7..b16e511 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -8073,6 +8073,23 @@ aarch64_set_current_function (tree fndecl) = save_target_globals_default_opts (); } } + + if (!fndecl) + return; + + /* If we turned on SIMD make sure that any vector parameters are re-laid out + so that they use proper vector modes. */ + if (TARGET_SIMD) + { + tree parms = DECL_ARGUMENTS (fndecl); + for (; parms && parms != void_list_node; parms = TREE_CHAIN (parms)) + { + if (TREE_CODE (parms) == PARM_DECL + && VECTOR_TYPE_P (TREE_TYPE (parms)) + && DECL_MODE (parms) != TYPE_MODE (TREE_TYPE (parms))) + relayout_decl (parms); + } + } } /* Enum describing the various ways we can handle attributes. diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_2.c b/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_2.c new file mode 100644 index 0000000..d6e7b68 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_2.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mcpu=thunderx+nofp" } */ + +/* Make sure that we don't ICE when dealing with vector parameters + in a simd-tagged function within a non-simd translation unit. */ + +#pragma GCC push_options +#pragma GCC target ("+nothing+simd") +typedef unsigned int __uint32_t; +typedef __uint32_t uint32_t ; +typedef __Uint32x4_t uint32x4_t; +#pragma GCC pop_options + + +__attribute__ ((target ("cpu=cortex-a57"))) +uint32x4_t +foo (uint32x4_t a, uint32_t b, uint32x4_t c) +{ + return c; +}