Re: [PATCH 1/6][ARM] Refactor NEON builtin framework to work for other builtins

2017-01-05 Thread Kyrill Tkachov

Hi Andre,

On 01/12/16 17:25, Andre Vieira (lists) wrote:

On 17/11/16 10:42, Kyrill Tkachov wrote:

Hi Andre,

On 09/11/16 10:11, Andre Vieira (lists) wrote:

Hi,

Refactor NEON builtin framework such that it can be used to implement
other builtins.

Is this OK for trunk?

Regards,
Andre

gcc/ChangeLog
2016-11-09  Andre Vieira  

  * config/arm/arm-builtins.c (neon_builtin_datum): Rename to ..
  (arm_builtin_datum): ... this.
  (arm_init_neon_builtin): Rename to ...
  (arm_init_builtin): ... this. Add a new parameters PREFIX
  and USE_SIG_IN_NAME.
  (arm_init_neon_builtins): Replace 'arm_init_neon_builtin' with
  'arm_init_builtin'. Replace type 'neon_builtin_datum' with
  'arm_builtin_datum'.
  (arm_init_vfp_builtins): Likewise.
  (builtin_arg): Rename enum's replacing 'NEON_ARG' with
  'ARG_BUILTIN' and add a 'ARG_BUILTIN_NEON_MEMORY.
  (arm_expand_neon_args): Rename to ...
  (arm_expand_builtin_args): ... this. Rename builtin_arg
  enum values and differentiate between ARG_BUILTIN_MEMORY
  and ARG_BUILTIN_NEON_MEMORY.
  (arm_expand_neon_builtin_1): Rename to ...
  (arm_expand_builtin_1): ... this. Rename builtin_arg enum
  values, arm_expand_builtin_args and add bool parameter NEON.
  (arm_expand_neon_builtin): Use arm_expand_builtin_1.
  (arm_expand_vfp_builtin): Likewise.
  (NEON_MAX_BUILTIN_ARGS): Remove, it was unused.

  /* Expand a neon builtin.  This is also used for vfp builtins, which
behave in
 the same way.  These builtins are "special" because they don't have
symbolic
 constants defined per-instruction or per instruction-variant.
Instead, the
-   required info is looked up in the NEON_BUILTIN_DATA record that is
passed
+   required info is looked up in the ARM_BUILTIN_DATA record that is
passed
 into the function.  */
  


The comment should be updated now that it's not just NEON builtins that
are expanded through this function.

  static rtx
-arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target,
-   neon_builtin_datum *d)
+arm_expand_builtin_1 (int fcode, tree exp, rtx target,
+   arm_builtin_datum *d, bool neon)
  {

I'm not a fan of this 'neon' boolean as it can cause confusion among the
users of the function
(see long thread at https://gcc.gnu.org/ml/gcc/2016-10/msg4.html).
Whether the builtin is a NEON/VFP builtin
can be distinguished from FCODE, so lets just make that bool neon a
local variable and initialise it accordingly
from FCODE.

Same for:
+/* Set up a builtin.  It will use information stored in the argument
struct D to
+   derive the builtin's type signature and name.  It will append the
name in D
+   to the PREFIX passed and use these to create a builtin declaration
that is
+   then stored in 'arm_builtin_decls' under index FCODE.  This FCODE is
also
+   written back to D for future use.  If USE_SIG_IN_NAME is true the
builtin's
+   name is appended with type signature information to distinguish between
+   signedness and poly.  */
  
  static void

-arm_init_neon_builtin (unsigned int fcode,
-   neon_builtin_datum *d)
+arm_init_builtin (unsigned int fcode, arm_builtin_datum *d,
+  const char * prefix, bool use_sig_in_name)

use_sig_in_name is dependent on FCODE so just deduce it from that
locally in arm_init_builtin.

This is ok otherwise.
Thanks,
Kyrill



Hi,

Reworked patch according to comments. No changes to ChangeLog.

Is this OK?


Ok. Please wait for the other patches in the series to be approved before 
committing.

Thanks,
Kyrill


Cheers,
Andre




Re: [PATCH 1/6][ARM] Refactor NEON builtin framework to work for other builtins

2016-12-01 Thread Andre Vieira (lists)
On 17/11/16 10:42, Kyrill Tkachov wrote:
> Hi Andre,
> 
> On 09/11/16 10:11, Andre Vieira (lists) wrote:
>> Hi,
>>
>> Refactor NEON builtin framework such that it can be used to implement
>> other builtins.
>>
>> Is this OK for trunk?
>>
>> Regards,
>> Andre
>>
>> gcc/ChangeLog
>> 2016-11-09  Andre Vieira  
>>
>>  * config/arm/arm-builtins.c (neon_builtin_datum): Rename to ..
>>  (arm_builtin_datum): ... this.
>>  (arm_init_neon_builtin): Rename to ...
>>  (arm_init_builtin): ... this. Add a new parameters PREFIX
>>  and USE_SIG_IN_NAME.
>>  (arm_init_neon_builtins): Replace 'arm_init_neon_builtin' with
>>  'arm_init_builtin'. Replace type 'neon_builtin_datum' with
>>  'arm_builtin_datum'.
>>  (arm_init_vfp_builtins): Likewise.
>>  (builtin_arg): Rename enum's replacing 'NEON_ARG' with
>>  'ARG_BUILTIN' and add a 'ARG_BUILTIN_NEON_MEMORY.
>>  (arm_expand_neon_args): Rename to ...
>>  (arm_expand_builtin_args): ... this. Rename builtin_arg
>>  enum values and differentiate between ARG_BUILTIN_MEMORY
>>  and ARG_BUILTIN_NEON_MEMORY.
>>  (arm_expand_neon_builtin_1): Rename to ...
>>  (arm_expand_builtin_1): ... this. Rename builtin_arg enum
>>  values, arm_expand_builtin_args and add bool parameter NEON.
>>  (arm_expand_neon_builtin): Use arm_expand_builtin_1.
>>  (arm_expand_vfp_builtin): Likewise.
>>  (NEON_MAX_BUILTIN_ARGS): Remove, it was unused.
> 
>  /* Expand a neon builtin.  This is also used for vfp builtins, which
> behave in
> the same way.  These builtins are "special" because they don't have
> symbolic
> constants defined per-instruction or per instruction-variant. 
> Instead, the
> -   required info is looked up in the NEON_BUILTIN_DATA record that is
> passed
> +   required info is looked up in the ARM_BUILTIN_DATA record that is
> passed
> into the function.  */
>  
> 
> The comment should be updated now that it's not just NEON builtins that
> are expanded through this function.
> 
>  static rtx
> -arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target,
> -   neon_builtin_datum *d)
> +arm_expand_builtin_1 (int fcode, tree exp, rtx target,
> +   arm_builtin_datum *d, bool neon)
>  {
> 
> I'm not a fan of this 'neon' boolean as it can cause confusion among the
> users of the function
> (see long thread at https://gcc.gnu.org/ml/gcc/2016-10/msg4.html).
> Whether the builtin is a NEON/VFP builtin
> can be distinguished from FCODE, so lets just make that bool neon a
> local variable and initialise it accordingly
> from FCODE.
> 
> Same for:
> +/* Set up a builtin.  It will use information stored in the argument
> struct D to
> +   derive the builtin's type signature and name.  It will append the
> name in D
> +   to the PREFIX passed and use these to create a builtin declaration
> that is
> +   then stored in 'arm_builtin_decls' under index FCODE.  This FCODE is
> also
> +   written back to D for future use.  If USE_SIG_IN_NAME is true the
> builtin's
> +   name is appended with type signature information to distinguish between
> +   signedness and poly.  */
>  
>  static void
> -arm_init_neon_builtin (unsigned int fcode,
> -   neon_builtin_datum *d)
> +arm_init_builtin (unsigned int fcode, arm_builtin_datum *d,
> +  const char * prefix, bool use_sig_in_name)
> 
> use_sig_in_name is dependent on FCODE so just deduce it from that
> locally in arm_init_builtin.
> 
> This is ok otherwise.
> Thanks,
> Kyrill
> 
> 

Hi,

Reworked patch according to comments. No changes to ChangeLog.

Is this OK?

Cheers,
Andre
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c
index 
5ed38d1608cfbfbd1248d76705fcf675bc36c2b2..da6331fdc729461adeb81d84c0c425bc45b80b8c
 100644
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -202,7 +202,7 @@ typedef struct {
   const enum insn_code code;
   unsigned int fcode;
   enum arm_type_qualifiers *qualifiers;
-} neon_builtin_datum;
+} arm_builtin_datum;
 
 #define CF(N,X) CODE_FOR_neon_##N##X
 
@@ -242,7 +242,7 @@ typedef struct {
   VAR11 (T, N, A, B, C, D, E, F, G, H, I, J, K) \
   VAR1 (T, N, L)
 
-/* The NEON builtin data can be found in arm_neon_builtins.def and
+/* The builtin data can be found in arm_neon_builtins.def,
arm_vfp_builtins.def.  The entries in arm_neon_builtins.def require
TARGET_NEON to be true.  The feature tests are checked when the
builtins are expanded.
@@ -252,14 +252,14 @@ typedef struct {
would be specified after the assembler mnemonic, which usually
refers to the last vector operand.  The modes listed per
instruction should be the same as those defined for that
-   instruction's pattern in neon.md.  */
+   instruction's pattern, for instance in neon.md.  */
 
-static neon_builtin_datum vfp_builtin_data[] =
+static arm_builtin_datum vfp_builtin_data[] =
 {
 #include "arm_vfp_builtins.def"
 };
 
-static 

Re: [PATCH 1/6][ARM] Refactor NEON builtin framework to work for other builtins

2016-11-17 Thread Kyrill Tkachov

Hi Andre,

On 09/11/16 10:11, Andre Vieira (lists) wrote:

Hi,

Refactor NEON builtin framework such that it can be used to implement
other builtins.

Is this OK for trunk?

Regards,
Andre

gcc/ChangeLog
2016-11-09  Andre Vieira  

 * config/arm/arm-builtins.c (neon_builtin_datum): Rename to ..
 (arm_builtin_datum): ... this.
 (arm_init_neon_builtin): Rename to ...
 (arm_init_builtin): ... this. Add a new parameters PREFIX
 and USE_SIG_IN_NAME.
 (arm_init_neon_builtins): Replace 'arm_init_neon_builtin' with
 'arm_init_builtin'. Replace type 'neon_builtin_datum' with
 'arm_builtin_datum'.
 (arm_init_vfp_builtins): Likewise.
 (builtin_arg): Rename enum's replacing 'NEON_ARG' with
 'ARG_BUILTIN' and add a 'ARG_BUILTIN_NEON_MEMORY.
 (arm_expand_neon_args): Rename to ...
 (arm_expand_builtin_args): ... this. Rename builtin_arg
 enum values and differentiate between ARG_BUILTIN_MEMORY
 and ARG_BUILTIN_NEON_MEMORY.
 (arm_expand_neon_builtin_1): Rename to ...
 (arm_expand_builtin_1): ... this. Rename builtin_arg enum
 values, arm_expand_builtin_args and add bool parameter NEON.
 (arm_expand_neon_builtin): Use arm_expand_builtin_1.
 (arm_expand_vfp_builtin): Likewise.
 (NEON_MAX_BUILTIN_ARGS): Remove, it was unused.


 /* Expand a neon builtin.  This is also used for vfp builtins, which behave in
the same way.  These builtins are "special" because they don't have symbolic
constants defined per-instruction or per instruction-variant.  Instead, the
-   required info is looked up in the NEON_BUILTIN_DATA record that is passed
+   required info is looked up in the ARM_BUILTIN_DATA record that is passed
into the function.  */
 


The comment should be updated now that it's not just NEON builtins that are 
expanded through this function.

 static rtx
-arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target,
-  neon_builtin_datum *d)
+arm_expand_builtin_1 (int fcode, tree exp, rtx target,
+  arm_builtin_datum *d, bool neon)
 {

I'm not a fan of this 'neon' boolean as it can cause confusion among the users 
of the function
(see long thread at https://gcc.gnu.org/ml/gcc/2016-10/msg4.html). Whether 
the builtin is a NEON/VFP builtin
can be distinguished from FCODE, so lets just make that bool neon a local 
variable and initialise it accordingly
from FCODE.

Same for:
+/* Set up a builtin.  It will use information stored in the argument struct D 
to
+   derive the builtin's type signature and name.  It will append the name in D
+   to the PREFIX passed and use these to create a builtin declaration that is
+   then stored in 'arm_builtin_decls' under index FCODE.  This FCODE is also
+   written back to D for future use.  If USE_SIG_IN_NAME is true the builtin's
+   name is appended with type signature information to distinguish between
+   signedness and poly.  */
 
 static void

-arm_init_neon_builtin (unsigned int fcode,
-  neon_builtin_datum *d)
+arm_init_builtin (unsigned int fcode, arm_builtin_datum *d,
+ const char * prefix, bool use_sig_in_name)

use_sig_in_name is dependent on FCODE so just deduce it from that locally in 
arm_init_builtin.

This is ok otherwise.
Thanks,
Kyrill