public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
@ 2023-09-20 13:09 Lehua Ding
  2023-09-27 22:24 ` Jeff Law
  2023-10-31  6:48 ` Andrew Pinski
  0 siblings, 2 replies; 7+ messages in thread
From: Lehua Ding @ 2023-09-20 13:09 UTC (permalink / raw)
  To: gcc-patches, richard.sandiford
  Cc: richard.guenther, juzhe.zhong, jeffreyalaw, lehua.ding

This patch adds combine cond_len_op and vec_cond to cond_len_op like
cond_op.

gcc/ChangeLog:

	* gimple-match.h (gimple_match_op::gimple_match_op):
	Add interfaces for more arguments.
	(gimple_match_op::set_op): Add interfaces for more arguments.
	* match.pd: Add support of combining cond_len_op + vec_cond
---
 gcc/gimple-match.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 gcc/match.pd       | 39 +++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h
index bec3ff42e3e..9892c142285 100644
--- a/gcc/gimple-match.h
+++ b/gcc/gimple-match.h
@@ -92,6 +92,10 @@ public:
 		   code_helper, tree, tree, tree, tree, tree);
   gimple_match_op (const gimple_match_cond &,
 		   code_helper, tree, tree, tree, tree, tree, tree);
+  gimple_match_op (const gimple_match_cond &,
+		   code_helper, tree, tree, tree, tree, tree, tree, tree);
+  gimple_match_op (const gimple_match_cond &,
+		   code_helper, tree, tree, tree, tree, tree, tree, tree, tree);
 
   void set_op (code_helper, tree, unsigned int);
   void set_op (code_helper, tree, tree);
@@ -100,6 +104,8 @@ public:
   void set_op (code_helper, tree, tree, tree, tree, bool);
   void set_op (code_helper, tree, tree, tree, tree, tree);
   void set_op (code_helper, tree, tree, tree, tree, tree, tree);
+  void set_op (code_helper, tree, tree, tree, tree, tree, tree, tree);
+  void set_op (code_helper, tree, tree, tree, tree, tree, tree, tree, tree);
   void set_value (tree);
 
   tree op_or_null (unsigned int) const;
@@ -212,6 +218,39 @@ gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
   ops[4] = op4;
 }
 
+inline
+gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
+				  code_helper code_in, tree type_in,
+				  tree op0, tree op1, tree op2, tree op3,
+				  tree op4, tree op5)
+  : cond (cond_in), code (code_in), type (type_in), reverse (false),
+    num_ops (6)
+{
+  ops[0] = op0;
+  ops[1] = op1;
+  ops[2] = op2;
+  ops[3] = op3;
+  ops[4] = op4;
+  ops[5] = op5;
+}
+
+inline
+gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
+				  code_helper code_in, tree type_in,
+				  tree op0, tree op1, tree op2, tree op3,
+				  tree op4, tree op5, tree op6)
+  : cond (cond_in), code (code_in), type (type_in), reverse (false),
+    num_ops (7)
+{
+  ops[0] = op0;
+  ops[1] = op1;
+  ops[2] = op2;
+  ops[3] = op3;
+  ops[4] = op4;
+  ops[5] = op5;
+  ops[6] = op6;
+}
+
 /* Change the operation performed to CODE_IN, the type of the result to
    TYPE_IN, and the number of operands to NUM_OPS_IN.  The caller needs
    to set the operands itself.  */
@@ -299,6 +338,39 @@ gimple_match_op::set_op (code_helper code_in, tree type_in,
   ops[4] = op4;
 }
 
+inline void
+gimple_match_op::set_op (code_helper code_in, tree type_in,
+			 tree op0, tree op1, tree op2, tree op3, tree op4,
+			 tree op5)
+{
+  code = code_in;
+  type = type_in;
+  num_ops = 6;
+  ops[0] = op0;
+  ops[1] = op1;
+  ops[2] = op2;
+  ops[3] = op3;
+  ops[4] = op4;
+  ops[5] = op5;
+}
+
+inline void
+gimple_match_op::set_op (code_helper code_in, tree type_in,
+			 tree op0, tree op1, tree op2, tree op3, tree op4,
+			 tree op5, tree op6)
+{
+  code = code_in;
+  type = type_in;
+  num_ops = 7;
+  ops[0] = op0;
+  ops[1] = op1;
+  ops[2] = op2;
+  ops[3] = op3;
+  ops[4] = op4;
+  ops[5] = op5;
+  ops[6] = op6;
+}
+
 /* Set the "operation" to be the single value VALUE, such as a constant
    or SSA_NAME.  */
 
diff --git a/gcc/match.pd b/gcc/match.pd
index a37af05f873..75b7e100120 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -103,12 +103,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   IFN_COND_FMIN IFN_COND_FMAX
   IFN_COND_AND IFN_COND_IOR IFN_COND_XOR
   IFN_COND_SHL IFN_COND_SHR)
+(define_operator_list COND_LEN_BINARY
+  IFN_COND_LEN_ADD IFN_COND_LEN_SUB
+  IFN_COND_LEN_MUL IFN_COND_LEN_DIV
+  IFN_COND_LEN_MOD IFN_COND_LEN_RDIV
+  IFN_COND_LEN_MIN IFN_COND_LEN_MAX
+  IFN_COND_LEN_FMIN IFN_COND_LEN_FMAX
+  IFN_COND_LEN_AND IFN_COND_LEN_IOR IFN_COND_LEN_XOR
+  IFN_COND_LEN_SHL IFN_COND_LEN_SHR)
 
 /* Same for ternary operations.  */
 (define_operator_list UNCOND_TERNARY
   IFN_FMA IFN_FMS IFN_FNMA IFN_FNMS)
 (define_operator_list COND_TERNARY
   IFN_COND_FMA IFN_COND_FMS IFN_COND_FNMA IFN_COND_FNMS)
+(define_operator_list COND_LEN_TERNARY
+  IFN_COND_LEN_FMA IFN_COND_LEN_FMS IFN_COND_LEN_FNMA IFN_COND_LEN_FNMS)
 
 /* __atomic_fetch_or_*, __atomic_fetch_xor_*, __atomic_xor_fetch_*  */
 (define_operator_list ATOMIC_FETCH_OR_XOR_N
@@ -8861,6 +8871,35 @@ and,
         && element_precision (type) == element_precision (op_type))
     (view_convert (cond_op @2 @3 @4 @5 (view_convert:op_type @1)))))))
 
+/* Detect cases in which a VEC_COND_EXPR effectively replaces the
+   "else" value of an IFN_COND_LEN_*.  */
+(for cond_len_op (COND_LEN_BINARY)
+ (simplify
+  (vec_cond @0 (view_convert? (cond_len_op @0 @1 @2 @3 @4 @5)) @6)
+  (with { tree op_type = TREE_TYPE (@3); }
+   (if (element_precision (type) == element_precision (op_type))
+    (view_convert (cond_len_op @0 @1 @2 (view_convert:op_type @6) @4 @5)))))
+ (simplify
+  (vec_cond @0 @1 (view_convert? (cond_len_op @2 @3 @4 @5 @6 @7)))
+  (with { tree op_type = TREE_TYPE (@5); }
+   (if (inverse_conditions_p (@0, @2)
+        && element_precision (type) == element_precision (op_type))
+    (view_convert (cond_len_op @2 @3 @4 (view_convert:op_type @1) @6 @7))))))
+
+/* Same for ternary operations.  */
+(for cond_len_op (COND_LEN_TERNARY)
+ (simplify
+  (vec_cond @0 (view_convert? (cond_len_op @0 @1 @2 @3 @4 @5 @6)) @7)
+  (with { tree op_type = TREE_TYPE (@4); }
+   (if (element_precision (type) == element_precision (op_type))
+    (view_convert (cond_len_op @0 @1 @2 @3 (view_convert:op_type @7) @5 @6)))))
+ (simplify
+  (vec_cond @0 @1 (view_convert? (cond_len_op @2 @3 @4 @5 @6 @7 @8)))
+  (with { tree op_type = TREE_TYPE (@6); }
+   (if (inverse_conditions_p (@0, @2)
+        && element_precision (type) == element_precision (op_type))
+    (view_convert (cond_len_op @2 @3 @4 @5 (view_convert:op_type @1) @7 @8))))))
+
 /* Detect simplication for a conditional reduction where
 
    a = mask1 ? b : 0
-- 
2.36.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
  2023-09-20 13:09 [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op Lehua Ding
@ 2023-09-27 22:24 ` Jeff Law
  2023-10-31  6:19   ` Lehua Ding
  2023-10-31  6:48 ` Andrew Pinski
  1 sibling, 1 reply; 7+ messages in thread
From: Jeff Law @ 2023-09-27 22:24 UTC (permalink / raw)
  To: Lehua Ding, gcc-patches, richard.sandiford; +Cc: richard.guenther, juzhe.zhong



On 9/20/23 07:09, Lehua Ding wrote:
> This patch adds combine cond_len_op and vec_cond to cond_len_op like
> cond_op.
> 
> gcc/ChangeLog:
> 
> 	* gimple-match.h (gimple_match_op::gimple_match_op):
> 	Add interfaces for more arguments.
> 	(gimple_match_op::set_op): Add interfaces for more arguments.
> 	* match.pd: Add support of combining cond_len_op + vec_cond
OK
jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
  2023-09-27 22:24 ` Jeff Law
@ 2023-10-31  6:19   ` Lehua Ding
  0 siblings, 0 replies; 7+ messages in thread
From: Lehua Ding @ 2023-10-31  6:19 UTC (permalink / raw)
  To: Jeff Law, gcc-patches, richard.sandiford; +Cc: richard.guenther, juzhe.zhong

Committed, thanks Jeff.

On 2023/9/28 6:24, Jeff Law wrote:
> 
> 
> On 9/20/23 07:09, Lehua Ding wrote:
>> This patch adds combine cond_len_op and vec_cond to cond_len_op like
>> cond_op.
>>
>> gcc/ChangeLog:
>>
>>     * gimple-match.h (gimple_match_op::gimple_match_op):
>>     Add interfaces for more arguments.
>>     (gimple_match_op::set_op): Add interfaces for more arguments.
>>     * match.pd: Add support of combining cond_len_op + vec_cond
> OK
> jeff
> 

-- 
Best,
Lehua (RiVAI)
lehua.ding@rivai.ai


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
  2023-09-20 13:09 [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op Lehua Ding
  2023-09-27 22:24 ` Jeff Law
@ 2023-10-31  6:48 ` Andrew Pinski
  2023-10-31  7:08   ` Lehua Ding
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Pinski @ 2023-10-31  6:48 UTC (permalink / raw)
  To: Lehua Ding
  Cc: gcc-patches, richard.sandiford, richard.guenther, juzhe.zhong,
	jeffreyalaw

On Wed, Sep 20, 2023 at 6:10 AM Lehua Ding <lehua.ding@rivai.ai> wrote:
>
> This patch adds combine cond_len_op and vec_cond to cond_len_op like
> cond_op.
>
> gcc/ChangeLog:
>
>         * gimple-match.h (gimple_match_op::gimple_match_op):
>         Add interfaces for more arguments.
>         (gimple_match_op::set_op): Add interfaces for more arguments.
>         * match.pd: Add support of combining cond_len_op + vec_cond
> ---
>  gcc/gimple-match.h | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>  gcc/match.pd       | 39 +++++++++++++++++++++++++
>  2 files changed, 111 insertions(+)
>
> diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h
> index bec3ff42e3e..9892c142285 100644
> --- a/gcc/gimple-match.h
> +++ b/gcc/gimple-match.h
> @@ -92,6 +92,10 @@ public:
>                    code_helper, tree, tree, tree, tree, tree);
>    gimple_match_op (const gimple_match_cond &,
>                    code_helper, tree, tree, tree, tree, tree, tree);
> +  gimple_match_op (const gimple_match_cond &,
> +                  code_helper, tree, tree, tree, tree, tree, tree, tree);
> +  gimple_match_op (const gimple_match_cond &,
> +                  code_helper, tree, tree, tree, tree, tree, tree, tree, tree);
>
>    void set_op (code_helper, tree, unsigned int);
>    void set_op (code_helper, tree, tree);
> @@ -100,6 +104,8 @@ public:
>    void set_op (code_helper, tree, tree, tree, tree, bool);
>    void set_op (code_helper, tree, tree, tree, tree, tree);
>    void set_op (code_helper, tree, tree, tree, tree, tree, tree);
> +  void set_op (code_helper, tree, tree, tree, tree, tree, tree, tree);
> +  void set_op (code_helper, tree, tree, tree, tree, tree, tree, tree, tree);
>    void set_value (tree);
>
>    tree op_or_null (unsigned int) const;
> @@ -212,6 +218,39 @@ gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
>    ops[4] = op4;
>  }
>
> +inline
> +gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
> +                                 code_helper code_in, tree type_in,
> +                                 tree op0, tree op1, tree op2, tree op3,
> +                                 tree op4, tree op5)
> +  : cond (cond_in), code (code_in), type (type_in), reverse (false),
> +    num_ops (6)
> +{
> +  ops[0] = op0;
> +  ops[1] = op1;
> +  ops[2] = op2;
> +  ops[3] = op3;
> +  ops[4] = op4;
> +  ops[5] = op5;
> +}

Hmm, does it make sense to start to use variadic templates for these
constructors instead of writing them out?
And we can even add a static_assert to make sure the number of
arguments is <= MAX_NUM_OPS to make sure they are correct. And use
std::is_same to make sure we are only passing tree types.

Thanks,
Andrew

> +
> +inline
> +gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
> +                                 code_helper code_in, tree type_in,
> +                                 tree op0, tree op1, tree op2, tree op3,
> +                                 tree op4, tree op5, tree op6)
> +  : cond (cond_in), code (code_in), type (type_in), reverse (false),
> +    num_ops (7)
> +{
> +  ops[0] = op0;
> +  ops[1] = op1;
> +  ops[2] = op2;
> +  ops[3] = op3;
> +  ops[4] = op4;
> +  ops[5] = op5;
> +  ops[6] = op6;
> +}
> +
>  /* Change the operation performed to CODE_IN, the type of the result to
>     TYPE_IN, and the number of operands to NUM_OPS_IN.  The caller needs
>     to set the operands itself.  */
> @@ -299,6 +338,39 @@ gimple_match_op::set_op (code_helper code_in, tree type_in,
>    ops[4] = op4;
>  }
>
> +inline void
> +gimple_match_op::set_op (code_helper code_in, tree type_in,
> +                        tree op0, tree op1, tree op2, tree op3, tree op4,
> +                        tree op5)
> +{
> +  code = code_in;
> +  type = type_in;
> +  num_ops = 6;
> +  ops[0] = op0;
> +  ops[1] = op1;
> +  ops[2] = op2;
> +  ops[3] = op3;
> +  ops[4] = op4;
> +  ops[5] = op5;
> +}
> +
> +inline void
> +gimple_match_op::set_op (code_helper code_in, tree type_in,
> +                        tree op0, tree op1, tree op2, tree op3, tree op4,
> +                        tree op5, tree op6)
> +{
> +  code = code_in;
> +  type = type_in;
> +  num_ops = 7;
> +  ops[0] = op0;
> +  ops[1] = op1;
> +  ops[2] = op2;
> +  ops[3] = op3;
> +  ops[4] = op4;
> +  ops[5] = op5;
> +  ops[6] = op6;
> +}
> +
>  /* Set the "operation" to be the single value VALUE, such as a constant
>     or SSA_NAME.  */
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index a37af05f873..75b7e100120 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -103,12 +103,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    IFN_COND_FMIN IFN_COND_FMAX
>    IFN_COND_AND IFN_COND_IOR IFN_COND_XOR
>    IFN_COND_SHL IFN_COND_SHR)
> +(define_operator_list COND_LEN_BINARY
> +  IFN_COND_LEN_ADD IFN_COND_LEN_SUB
> +  IFN_COND_LEN_MUL IFN_COND_LEN_DIV
> +  IFN_COND_LEN_MOD IFN_COND_LEN_RDIV
> +  IFN_COND_LEN_MIN IFN_COND_LEN_MAX
> +  IFN_COND_LEN_FMIN IFN_COND_LEN_FMAX
> +  IFN_COND_LEN_AND IFN_COND_LEN_IOR IFN_COND_LEN_XOR
> +  IFN_COND_LEN_SHL IFN_COND_LEN_SHR)
>
>  /* Same for ternary operations.  */
>  (define_operator_list UNCOND_TERNARY
>    IFN_FMA IFN_FMS IFN_FNMA IFN_FNMS)
>  (define_operator_list COND_TERNARY
>    IFN_COND_FMA IFN_COND_FMS IFN_COND_FNMA IFN_COND_FNMS)
> +(define_operator_list COND_LEN_TERNARY
> +  IFN_COND_LEN_FMA IFN_COND_LEN_FMS IFN_COND_LEN_FNMA IFN_COND_LEN_FNMS)
>
>  /* __atomic_fetch_or_*, __atomic_fetch_xor_*, __atomic_xor_fetch_*  */
>  (define_operator_list ATOMIC_FETCH_OR_XOR_N
> @@ -8861,6 +8871,35 @@ and,
>          && element_precision (type) == element_precision (op_type))
>      (view_convert (cond_op @2 @3 @4 @5 (view_convert:op_type @1)))))))
>
> +/* Detect cases in which a VEC_COND_EXPR effectively replaces the
> +   "else" value of an IFN_COND_LEN_*.  */
> +(for cond_len_op (COND_LEN_BINARY)
> + (simplify
> +  (vec_cond @0 (view_convert? (cond_len_op @0 @1 @2 @3 @4 @5)) @6)
> +  (with { tree op_type = TREE_TYPE (@3); }
> +   (if (element_precision (type) == element_precision (op_type))
> +    (view_convert (cond_len_op @0 @1 @2 (view_convert:op_type @6) @4 @5)))))
> + (simplify
> +  (vec_cond @0 @1 (view_convert? (cond_len_op @2 @3 @4 @5 @6 @7)))
> +  (with { tree op_type = TREE_TYPE (@5); }
> +   (if (inverse_conditions_p (@0, @2)
> +        && element_precision (type) == element_precision (op_type))
> +    (view_convert (cond_len_op @2 @3 @4 (view_convert:op_type @1) @6 @7))))))
> +
> +/* Same for ternary operations.  */
> +(for cond_len_op (COND_LEN_TERNARY)
> + (simplify
> +  (vec_cond @0 (view_convert? (cond_len_op @0 @1 @2 @3 @4 @5 @6)) @7)
> +  (with { tree op_type = TREE_TYPE (@4); }
> +   (if (element_precision (type) == element_precision (op_type))
> +    (view_convert (cond_len_op @0 @1 @2 @3 (view_convert:op_type @7) @5 @6)))))
> + (simplify
> +  (vec_cond @0 @1 (view_convert? (cond_len_op @2 @3 @4 @5 @6 @7 @8)))
> +  (with { tree op_type = TREE_TYPE (@6); }
> +   (if (inverse_conditions_p (@0, @2)
> +        && element_precision (type) == element_precision (op_type))
> +    (view_convert (cond_len_op @2 @3 @4 @5 (view_convert:op_type @1) @7 @8))))))
> +
>  /* Detect simplication for a conditional reduction where
>
>     a = mask1 ? b : 0
> --
> 2.36.3
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
  2023-10-31  6:48 ` Andrew Pinski
@ 2023-10-31  7:08   ` Lehua Ding
  2023-10-31  7:16     ` Andrew Pinski
  0 siblings, 1 reply; 7+ messages in thread
From: Lehua Ding @ 2023-10-31  7:08 UTC (permalink / raw)
  To: Andrew Pinski
  Cc: gcc-patches, richard.sandiford, richard.guenther, juzhe.zhong,
	jeffreyalaw

Hi Andrew,

On 2023/10/31 14:48, Andrew Pinski wrote:
>> +inline
>> +gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
>> +                                 code_helper code_in, tree type_in,
>> +                                 tree op0, tree op1, tree op2, tree op3,
>> +                                 tree op4, tree op5)
>> +  : cond (cond_in), code (code_in), type (type_in), reverse (false),
>> +    num_ops (6)
>> +{
>> +  ops[0] = op0;
>> +  ops[1] = op1;
>> +  ops[2] = op2;
>> +  ops[3] = op3;
>> +  ops[4] = op4;
>> +  ops[5] = op5;
>> +}
> Hmm, does it make sense to start to use variadic templates for these
> constructors instead of writing them out?
> And we can even add a static_assert to make sure the number of
> arguments is <= MAX_NUM_OPS to make sure they are correct. And use
> std::is_same to make sure we are only passing tree types.

You mean something like this?:

template<typename... op_types>
inline
gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
                                   code_helper code_in, tree type_in,
				  op_types... ops)
   : cond (cond_in), code (code_in), type (type_in), reverse (false),
     num_ops (sizeof...(ops))
{
   static_assert (sizeof...(ops) <= MAX_NUM_OPS);
   auto op_list[] = {ops...};
   for (int i = 0; i < sizeof...(ops); i++)
     this->ops[i] = op_list[i];
}

-- 
Best,
Lehua (RiVAI)
lehua.ding@rivai.ai

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
  2023-10-31  7:08   ` Lehua Ding
@ 2023-10-31  7:16     ` Andrew Pinski
  2023-10-31  8:20       ` Lehua Ding
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Pinski @ 2023-10-31  7:16 UTC (permalink / raw)
  To: Lehua Ding
  Cc: gcc-patches, richard.sandiford, richard.guenther, juzhe.zhong,
	jeffreyalaw

On Tue, Oct 31, 2023 at 12:08 AM Lehua Ding <lehua.ding@rivai.ai> wrote:
>
> Hi Andrew,
>
> On 2023/10/31 14:48, Andrew Pinski wrote:
> >> +inline
> >> +gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
> >> +                                 code_helper code_in, tree type_in,
> >> +                                 tree op0, tree op1, tree op2, tree op3,
> >> +                                 tree op4, tree op5)
> >> +  : cond (cond_in), code (code_in), type (type_in), reverse (false),
> >> +    num_ops (6)
> >> +{
> >> +  ops[0] = op0;
> >> +  ops[1] = op1;
> >> +  ops[2] = op2;
> >> +  ops[3] = op3;
> >> +  ops[4] = op4;
> >> +  ops[5] = op5;
> >> +}
> > Hmm, does it make sense to start to use variadic templates for these
> > constructors instead of writing them out?
> > And we can even add a static_assert to make sure the number of
> > arguments is <= MAX_NUM_OPS to make sure they are correct. And use
> > std::is_same to make sure we are only passing tree types.
>
> You mean something like this?:
>
> template<typename... op_types>
> inline
> gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
>                                    code_helper code_in, tree type_in,
>                                   op_types... ops)
>    : cond (cond_in), code (code_in), type (type_in), reverse (false),
>      num_ops (sizeof...(ops))
> {
>    static_assert (sizeof...(ops) <= MAX_NUM_OPS);
>    auto op_list[] = {ops...};
>    for (int i = 0; i < sizeof...(ops); i++)
>      this->ops[i] = op_list[i];
> }

Yes and maybe use tree for the type of op_list instead of auto.
I suspect this code was originally written before GCC was written in C++11.
Maybe if this code is being compiled with C++20 we could do something like:
#include <concepts>
template< std::same_as<tree>... op_types>

To get a decent error message earlier ...

Thanks,
Andrew

>
> --
> Best,
> Lehua (RiVAI)
> lehua.ding@rivai.ai

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
  2023-10-31  7:16     ` Andrew Pinski
@ 2023-10-31  8:20       ` Lehua Ding
  0 siblings, 0 replies; 7+ messages in thread
From: Lehua Ding @ 2023-10-31  8:20 UTC (permalink / raw)
  To: Andrew Pinski
  Cc: gcc-patches, richard.sandiford, richard.guenther, juzhe.zhong,
	jeffreyalaw

Hi Andrew,

> Yes and maybe use tree for the type of op_list instead of auto.
> I suspect this code was originally written before GCC was written in C++11.
> Maybe if this code is being compiled with C++20 we could do something like:
> #include <concepts>
> template< std::same_as<tree>... op_types>
> 
> To get a decent error message earlier ...

Or I think it's easier to understand without using a template by 
changing it to the following:

inline
gimple_match_op::gimple_match_op (const gimple_match_cond &cond_in,
				  code_helper code_in, tree type_in,
				  tree ops[], int num_op)
{
   for (int i = 0; i < num_op)
     this->ops[i] = ops[i];
}

-- 
Best,
Lehua (RiVAI)
lehua.ding@rivai.ai


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-10-31  8:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-20 13:09 [PATCH 1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op Lehua Ding
2023-09-27 22:24 ` Jeff Law
2023-10-31  6:19   ` Lehua Ding
2023-10-31  6:48 ` Andrew Pinski
2023-10-31  7:08   ` Lehua Ding
2023-10-31  7:16     ` Andrew Pinski
2023-10-31  8:20       ` Lehua Ding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).