[1/2] match.pd: Support combine cond_len_op + vec_cond similar to cond_op
Checks
Commit Message
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(+)
Comments
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
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
>
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
>
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];
}
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
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];
}
@@ -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. */
@@ -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