diff mbox

[AArch64,11/14] Re-layout SIMD builtin types on builtin expansion

Message ID 55B1F98A.3070307@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov July 24, 2015, 8:38 a.m. UTC 
On 22/07/15 10:11, James Greenhalgh wrote:
> 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, here's an updated version.

2015-07-24  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-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/aarch64/target_attr_crypto_ice_1.c: New test.

> Thanks,
> James

Comments

James Greenhalgh Aug. 3, 2015, 11:24 a.m. UTC | #1 
On Fri, Jul 24, 2015 at 09:38:34AM +0100, Kyrill Tkachov wrote:
> Thanks, here's an updated version.
> 
> 2015-07-24  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-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      * gcc.target/aarch64/target_attr_crypto_ice_1.c: New test.
> 

OK with a minor fix.

> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 6844c90..99fd80e 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -255,6 +255,7 @@ bool aarch64_float_const_zero_rtx_p (rtx);
>  bool aarch64_function_arg_regno_p (unsigned);
>  bool aarch64_gen_movmemqi (rtx *);
>  bool aarch64_gimple_fold_builtin (gimple_stmt_iterator *);
> +void aarch64_init_simd_builtins (void);
>  bool aarch64_is_extend_from_extract (machine_mode, rtx, rtx);
>  bool aarch64_is_long_call_p (rtx);
>  bool aarch64_label_mentioned_p (rtx);

These should be first ordered by return type, then alphabetical order.

> @@ -325,6 +326,7 @@ void aarch64_print_operand (FILE *, rtx, char);
>  void aarch64_print_operand_address (FILE *, rtx);
>  void aarch64_emit_call_insn (rtx);
>  void aarch64_register_pragmas (void);
> +void aarch64_relayout_simd_types (void);
>  void aarch64_reset_previous_fndecl (void);
>  
>  /* Initialize builtins for SIMD intrinsics.  */

Thanks,
James
diff mbox

Patch

commit 64ea339d84a269fdd7ff5c3ad733135e1f05b862
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed May 20 12:02:33 2015 +0100

    [AArch64][11/N] Re-layout SIMD builtin types on builtin expansion

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 4b78329..4ad7376 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -681,11 +681,18 @@  aarch64_init_simd_builtin_scalar_types (void)
 					     "__builtin_aarch64_simd_udi");
 }
 
-static void
+static bool aarch64_simd_builtins_initialized_p = false;
+
+void
 aarch64_init_simd_builtins (void)
 {
   unsigned int i, fcode = AARCH64_SIMD_PATTERN_START;
 
+  if (aarch64_simd_builtins_initialized_p)
+    return;
+
+  aarch64_simd_builtins_initialized_p = true;
+
   aarch64_init_simd_builtin_types ();
 
   /* Strong-typing hasn't been implemented for all AdvSIMD builtin intrinsics.
@@ -848,8 +855,8 @@  aarch64_init_builtins (void)
 
   if (TARGET_SIMD)
     aarch64_init_simd_builtins ();
-  if (TARGET_CRC32)
-    aarch64_init_crc32_builtins ();
+
+  aarch64_init_crc32_builtins ();
 }
 
 tree
@@ -870,6 +877,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.  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,
 			  tree exp, builtin_simd_arg *args,
@@ -899,6 +931,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 e5e8a1f..79378d8 100644
--- a/gcc/config/aarch64/aarch64-c.c
+++ b/gcc/config/aarch64/aarch64-c.c
@@ -174,6 +174,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 true;
 }
 
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 6844c90..99fd80e 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -255,6 +255,7 @@  bool aarch64_float_const_zero_rtx_p (rtx);
 bool aarch64_function_arg_regno_p (unsigned);
 bool aarch64_gen_movmemqi (rtx *);
 bool aarch64_gimple_fold_builtin (gimple_stmt_iterator *);
+void aarch64_init_simd_builtins (void);
 bool aarch64_is_extend_from_extract (machine_mode, rtx, rtx);
 bool aarch64_is_long_call_p (rtx);
 bool aarch64_label_mentioned_p (rtx);
@@ -325,6 +326,7 @@  void aarch64_print_operand (FILE *, rtx, char);
 void aarch64_print_operand_address (FILE *, rtx);
 void aarch64_emit_call_insn (rtx);
 void aarch64_register_pragmas (void);
+void aarch64_relayout_simd_types (void);
 void aarch64_reset_previous_fndecl (void);
 
 /* Initialize builtins for SIMD intrinsics.  */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 62cf9a2..334a681 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8474,6 +8474,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.  */
+      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;
+	}
       new_target = build_target_option_node (&global_options);
     }
   else
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_1.c b/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_1.c
new file mode 100644
index 0000000..42f14c4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_1.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=thunderx+nofp" } */
+
+#include "arm_neon.h"
+
+/* Unless we do something about re-laying out the SIMD builtin types
+   this testcase ICEs during expansion of the crypto builtin.  */
+
+__attribute__ ((target ("cpu=cortex-a57+crypto")))
+uint32x4_t
+test_vsha1cq_u32 (uint32x4_t hash_abcd, uint32_t hash_e, uint32x4_t wk)
+{
+  return vsha1cq_u32 (hash_abcd, hash_e, wk);
+}
+
+/* This one should be compiled for thunderx with no fp.  */
+int
+foo (int a)
+{
+  return a + 5;
+}