* [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
@ 2024-05-19 4:16 pan2.li
2024-05-19 4:24 ` Andrew Pinski
2024-05-20 11:20 ` Tamar Christina
0 siblings, 2 replies; 5+ messages in thread
From: pan2.li @ 2024-05-19 4:16 UTC (permalink / raw)
To: gcc-patches
Cc: juzhe.zhong, kito.cheng, tamar.christina, richard.guenther, Pan Li
From: Pan Li <pan2.li@intel.com>
There are sorts of match pattern for SAT related cases, there will be
some duplicated code to check the dest, op_0, op_1 are same tree types.
Aka ternary tree type matches. Thus, extract one helper function to
do this and avoid match code duplication.
The below test suites are passed for this patch:
* The rv64gcv fully regression test.
* The x86 bootstrap test.
* The x86 regression test.
gcc/ChangeLog:
* generic-match-head.cc (integer_types_ternary_match): New helper
function to check tenary tree type matches or not.
* gimple-match-head.cc (integer_types_ternary_match): Ditto but
for match.
* match.pd: Leverage above helper function to avoid code dup.
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/generic-match-head.cc | 17 +++++++++++++++++
gcc/gimple-match-head.cc | 17 +++++++++++++++++
gcc/match.pd | 25 +++++--------------------
3 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
index 0d3f648fe8d..cdd48c7a5cc 100644
--- a/gcc/generic-match-head.cc
+++ b/gcc/generic-match-head.cc
@@ -59,6 +59,23 @@ types_match (tree t1, tree t2)
return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2);
}
+/* Routine to determine if the types T1, T2 and T3 are effectively
+ the same integer type for GENERIC. If T1, T2 or T3 is not a type,
+ the test applies to their TREE_TYPE. */
+
+static inline bool
+integer_types_ternary_match (tree t1, tree t2, tree t3)
+{
+ t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
+ t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
+ t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
+
+ if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P (t3))
+ return false;
+
+ return types_match (t1, t2) && types_match (t1, t3);
+}
+
/* Return if T has a single use. For GENERIC, we assume this is
always true. */
diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
index 5f8a1a1ad8e..91f2e56b8ef 100644
--- a/gcc/gimple-match-head.cc
+++ b/gcc/gimple-match-head.cc
@@ -79,6 +79,23 @@ types_match (tree t1, tree t2)
return types_compatible_p (t1, t2);
}
+/* Routine to determine if the types T1, T2 and T3 are effectively
+ the same integer type for GIMPLE. If T1, T2 or T3 is not a type,
+ the test applies to their TREE_TYPE. */
+
+static inline bool
+integer_types_ternary_match (tree t1, tree t2, tree t3)
+{
+ t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
+ t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
+ t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
+
+ if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P (t3))
+ return false;
+
+ return types_match (t1, t2) && types_match (t1, t3);
+}
+
/* Return if T has a single use. For GIMPLE, we also allow any
non-SSA_NAME (ie constants) and zero uses to cope with uses
that aren't linked up yet. */
diff --git a/gcc/match.pd b/gcc/match.pd
index 0f9c34fa897..b291e34bbe4 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3046,38 +3046,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* Unsigned Saturation Add */
(match (usadd_left_part_1 @0 @1)
(plus:c @0 @1)
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_left_part_2 @0 @1)
(realpart (IFN_ADD_OVERFLOW:c @0 @1))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_right_part_1 @0 @1)
(negate (convert (lt (plus:c @0 @1) @0)))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_right_part_1 @0 @1)
(negate (convert (gt @0 (plus:c @0 @1))))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_right_part_2 @0 @1)
(negate (convert (ne (imagpart (IFN_ADD_OVERFLOW:c @0 @1)) integer_zerop)))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
/* We cannot merge or overload usadd_left_part_1 and usadd_left_part_2
because the sub part of left_part_2 cannot work with right_part_1.
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
2024-05-19 4:16 [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC] pan2.li
@ 2024-05-19 4:24 ` Andrew Pinski
2024-05-20 11:00 ` Li, Pan2
2024-05-20 11:20 ` Tamar Christina
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Pinski @ 2024-05-19 4:24 UTC (permalink / raw)
To: pan2.li
Cc: GCC Patches, 钟居哲,
Kito Cheng, Tamar Christina, Richard Guenther
[-- Attachment #1: Type: text/plain, Size: 5428 bytes --]
On Sat, May 18, 2024, 9:17 PM <pan2.li@intel.com> wrote:
> From: Pan Li <pan2.li@intel.com>
>
> There are sorts of match pattern for SAT related cases, there will be
> some duplicated code to check the dest, op_0, op_1 are same tree types.
> Aka ternary tree type matches. Thus, extract one helper function to
> do this and avoid match code duplication.
>
> The below test suites are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 regression test.
>
> gcc/ChangeLog:
>
> * generic-match-head.cc (integer_types_ternary_match): New helper
> function to check tenary tree type matches or not.
> * gimple-match-head.cc (integer_types_ternary_match): Ditto but
> for match.
> * match.pd: Leverage above helper function to avoid code dup.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/generic-match-head.cc | 17 +++++++++++++++++
> gcc/gimple-match-head.cc | 17 +++++++++++++++++
> gcc/match.pd | 25 +++++--------------------
> 3 files changed, 39 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
> index 0d3f648fe8d..cdd48c7a5cc 100644
> --- a/gcc/generic-match-head.cc
> +++ b/gcc/generic-match-head.cc
> @@ -59,6 +59,23 @@ types_match (tree t1, tree t2)
> return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2);
> }
>
> +/* Routine to determine if the types T1, T2 and T3 are effectively
> + the same integer type for GENERIC. If T1, T2 or T3 is not a type,
> + the test applies to their TREE_TYPE. */
> +
> +static inline bool
> +integer_types_ternary_match (tree t1, tree t2, tree t3)
> +{
> + t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
> + t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
> + t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
> +
> + if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P
> (t3))
> + return false;
> +
> + return types_match (t1, t2) && types_match (t1, t3);
> +}
> +
> /* Return if T has a single use. For GENERIC, we assume this is
> always true. */
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index 5f8a1a1ad8e..91f2e56b8ef 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -79,6 +79,23 @@ types_match (tree t1, tree t2)
> return types_compatible_p (t1, t2);
> }
>
> +/* Routine to determine if the types T1, T2 and T3 are effectively
> + the same integer type for GIMPLE. If T1, T2 or T3 is not a type,
> + the test applies to their TREE_TYPE. */
> +
> +static inline bool
> +integer_types_ternary_match (tree t1, tree t2, tree t3)
> +{
> + t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
> + t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
> + t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
> +
> + if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P
> (t3))
> + return false;
> +
> + return types_match (t1, t2) && types_match (t1, t3);
> +}
> +
> /* Return if T has a single use. For GIMPLE, we also allow any
> non-SSA_NAME (ie constants) and zero uses to cope with uses
> that aren't linked up yet. */
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 0f9c34fa897..b291e34bbe4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3046,38 +3046,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* Unsigned Saturation Add */
> (match (usadd_left_part_1 @0 @1)
> (plus:c @0 @1)
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0,
> @1))))
>
Even though unsigned might be the cheaper check, you might need to swap the
order back to where it was so you check integral first.
Otherwise this is nice cleanup. (Note I can't approve it though).
Thanks,
Andrew
> (match (usadd_left_part_2 @0 @1)
> (realpart (IFN_ADD_OVERFLOW:c @0 @1))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0,
> @1))))
>
> (match (usadd_right_part_1 @0 @1)
> (negate (convert (lt (plus:c @0 @1) @0)))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0,
> @1))))
>
> (match (usadd_right_part_1 @0 @1)
> (negate (convert (gt @0 (plus:c @0 @1))))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0,
> @1))))
>
> (match (usadd_right_part_2 @0 @1)
> (negate (convert (ne (imagpart (IFN_ADD_OVERFLOW:c @0 @1))
> integer_zerop)))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0,
> @1))))
>
> /* We cannot merge or overload usadd_left_part_1 and usadd_left_part_2
> because the sub part of left_part_2 cannot work with right_part_1.
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
2024-05-19 4:24 ` Andrew Pinski
@ 2024-05-20 11:00 ` Li, Pan2
0 siblings, 0 replies; 5+ messages in thread
From: Li, Pan2 @ 2024-05-20 11:00 UTC (permalink / raw)
To: Andrew Pinski
Cc: GCC Patches, 钟居哲,
Kito Cheng, Tamar Christina, Richard Guenther
[-- Attachment #1: Type: text/plain, Size: 5845 bytes --]
Thanks Andrew for comments, updated in v2.
Pan
From: Andrew Pinski <pinskia@gmail.com>
Sent: Sunday, May 19, 2024 12:25 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>; 钟居哲 <juzhe.zhong@rivai.ai>; Kito Cheng <kito.cheng@gmail.com>; Tamar Christina <tamar.christina@arm.com>; Richard Guenther <richard.guenther@gmail.com>
Subject: Re: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
On Sat, May 18, 2024, 9:17 PM <pan2.li@intel.com<mailto:pan2.li@intel.com>> wrote:
From: Pan Li <pan2.li@intel.com<mailto:pan2.li@intel.com>>
There are sorts of match pattern for SAT related cases, there will be
some duplicated code to check the dest, op_0, op_1 are same tree types.
Aka ternary tree type matches. Thus, extract one helper function to
do this and avoid match code duplication.
The below test suites are passed for this patch:
* The rv64gcv fully regression test.
* The x86 bootstrap test.
* The x86 regression test.
gcc/ChangeLog:
* generic-match-head.cc (integer_types_ternary_match): New helper
function to check tenary tree type matches or not.
* gimple-match-head.cc (integer_types_ternary_match): Ditto but
for match.
* match.pd: Leverage above helper function to avoid code dup.
Signed-off-by: Pan Li <pan2.li@intel.com<mailto:pan2.li@intel.com>>
---
gcc/generic-match-head.cc | 17 +++++++++++++++++
gcc/gimple-match-head.cc | 17 +++++++++++++++++
gcc/match.pd | 25 +++++--------------------
3 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
index 0d3f648fe8d..cdd48c7a5cc 100644
--- a/gcc/generic-match-head.cc
+++ b/gcc/generic-match-head.cc
@@ -59,6 +59,23 @@ types_match (tree t1, tree t2)
return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2);
}
+/* Routine to determine if the types T1, T2 and T3 are effectively
+ the same integer type for GENERIC. If T1, T2 or T3 is not a type,
+ the test applies to their TREE_TYPE. */
+
+static inline bool
+integer_types_ternary_match (tree t1, tree t2, tree t3)
+{
+ t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
+ t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
+ t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
+
+ if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P (t3))
+ return false;
+
+ return types_match (t1, t2) && types_match (t1, t3);
+}
+
/* Return if T has a single use. For GENERIC, we assume this is
always true. */
diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
index 5f8a1a1ad8e..91f2e56b8ef 100644
--- a/gcc/gimple-match-head.cc
+++ b/gcc/gimple-match-head.cc
@@ -79,6 +79,23 @@ types_match (tree t1, tree t2)
return types_compatible_p (t1, t2);
}
+/* Routine to determine if the types T1, T2 and T3 are effectively
+ the same integer type for GIMPLE. If T1, T2 or T3 is not a type,
+ the test applies to their TREE_TYPE. */
+
+static inline bool
+integer_types_ternary_match (tree t1, tree t2, tree t3)
+{
+ t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
+ t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
+ t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
+
+ if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P (t3))
+ return false;
+
+ return types_match (t1, t2) && types_match (t1, t3);
+}
+
/* Return if T has a single use. For GIMPLE, we also allow any
non-SSA_NAME (ie constants) and zero uses to cope with uses
that aren't linked up yet. */
diff --git a/gcc/match.pd b/gcc/match.pd
index 0f9c34fa897..b291e34bbe4 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3046,38 +3046,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* Unsigned Saturation Add */
(match (usadd_left_part_1 @0 @1)
(plus:c @0 @1)
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
Even though unsigned might be the cheaper check, you might need to swap the order back to where it was so you check integral first.
Otherwise this is nice cleanup. (Note I can't approve it though).
Thanks,
Andrew
(match (usadd_left_part_2 @0 @1)
(realpart (IFN_ADD_OVERFLOW:c @0 @1))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_right_part_1 @0 @1)
(negate (convert (lt (plus:c @0 @1) @0)))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_right_part_1 @0 @1)
(negate (convert (gt @0 (plus:c @0 @1))))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
(match (usadd_right_part_2 @0 @1)
(negate (convert (ne (imagpart (IFN_ADD_OVERFLOW:c @0 @1)) integer_zerop)))
- (if (INTEGRAL_TYPE_P (type)
- && TYPE_UNSIGNED (TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@0))
- && types_match (type, TREE_TYPE (@1)))))
+ (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
/* We cannot merge or overload usadd_left_part_1 and usadd_left_part_2
because the sub part of left_part_2 cannot work with right_part_1.
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
2024-05-19 4:16 [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC] pan2.li
2024-05-19 4:24 ` Andrew Pinski
@ 2024-05-20 11:20 ` Tamar Christina
2024-05-20 11:48 ` Li, Pan2
1 sibling, 1 reply; 5+ messages in thread
From: Tamar Christina @ 2024-05-20 11:20 UTC (permalink / raw)
To: pan2.li, gcc-patches; +Cc: juzhe.zhong, kito.cheng, richard.guenther
> -----Original Message-----
> From: pan2.li@intel.com <pan2.li@intel.com>
> Sent: Sunday, May 19, 2024 5:17 AM
> To: gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; kito.cheng@gmail.com; Tamar Christina
> <Tamar.Christina@arm.com>; richard.guenther@gmail.com; Pan Li
> <pan2.li@intel.com>
> Subject: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid
> code dup [NFC]
>
> From: Pan Li <pan2.li@intel.com>
>
> There are sorts of match pattern for SAT related cases, there will be
> some duplicated code to check the dest, op_0, op_1 are same tree types.
> Aka ternary tree type matches. Thus, extract one helper function to
> do this and avoid match code duplication.
>
> The below test suites are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 regression test.
>
> gcc/ChangeLog:
>
> * generic-match-head.cc (integer_types_ternary_match): New helper
> function to check tenary tree type matches or not.
> * gimple-match-head.cc (integer_types_ternary_match): Ditto but
> for match.
> * match.pd: Leverage above helper function to avoid code dup.
>
Nice cleanup!
This function isn't part of the machinery of match.pd and is instead part of a pattern.
For these things we usually put them in tree.h/tree.cc and declare them at the top
of match.pd in the "define_predicates" list.
This will also allow you to get rid of the code duplication. In addition such functions
which return a true/false we consider predicates, and name them ending with _p.
See e.g. bitmask_inv_cst_vector_p which is also defined in tree.h/tree.cc.
Cheers,
Tamar
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/generic-match-head.cc | 17 +++++++++++++++++
> gcc/gimple-match-head.cc | 17 +++++++++++++++++
> gcc/match.pd | 25 +++++--------------------
> 3 files changed, 39 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
> index 0d3f648fe8d..cdd48c7a5cc 100644
> --- a/gcc/generic-match-head.cc
> +++ b/gcc/generic-match-head.cc
> @@ -59,6 +59,23 @@ types_match (tree t1, tree t2)
> return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2);
> }
>
> +/* Routine to determine if the types T1, T2 and T3 are effectively
> + the same integer type for GENERIC. If T1, T2 or T3 is not a type,
> + the test applies to their TREE_TYPE. */
> +
> +static inline bool
> +integer_types_ternary_match (tree t1, tree t2, tree t3)
> +{
> + t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
> + t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
> + t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
> +
> + if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P
> (t3))
> + return false;
> +
> + return types_match (t1, t2) && types_match (t1, t3);
> +}
> +
> /* Return if T has a single use. For GENERIC, we assume this is
> always true. */
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index 5f8a1a1ad8e..91f2e56b8ef 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -79,6 +79,23 @@ types_match (tree t1, tree t2)
> return types_compatible_p (t1, t2);
> }
>
> +/* Routine to determine if the types T1, T2 and T3 are effectively
> + the same integer type for GIMPLE. If T1, T2 or T3 is not a type,
> + the test applies to their TREE_TYPE. */
> +
> +static inline bool
> +integer_types_ternary_match (tree t1, tree t2, tree t3)
> +{
> + t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
> + t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
> + t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
> +
> + if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P
> (t3))
> + return false;
> +
> + return types_match (t1, t2) && types_match (t1, t3);
> +}
> +
> /* Return if T has a single use. For GIMPLE, we also allow any
> non-SSA_NAME (ie constants) and zero uses to cope with uses
> that aren't linked up yet. */
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 0f9c34fa897..b291e34bbe4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3046,38 +3046,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* Unsigned Saturation Add */
> (match (usadd_left_part_1 @0 @1)
> (plus:c @0 @1)
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_left_part_2 @0 @1)
> (realpart (IFN_ADD_OVERFLOW:c @0 @1))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_right_part_1 @0 @1)
> (negate (convert (lt (plus:c @0 @1) @0)))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_right_part_1 @0 @1)
> (negate (convert (gt @0 (plus:c @0 @1))))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_right_part_2 @0 @1)
> (negate (convert (ne (imagpart (IFN_ADD_OVERFLOW:c @0 @1))
> integer_zerop)))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> /* We cannot merge or overload usadd_left_part_1 and usadd_left_part_2
> because the sub part of left_part_2 cannot work with right_part_1.
> --
> 2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
2024-05-20 11:20 ` Tamar Christina
@ 2024-05-20 11:48 ` Li, Pan2
0 siblings, 0 replies; 5+ messages in thread
From: Li, Pan2 @ 2024-05-20 11:48 UTC (permalink / raw)
To: Tamar Christina, gcc-patches; +Cc: juzhe.zhong, kito.cheng, richard.guenther
> See e.g. bitmask_inv_cst_vector_p which is also defined in tree.h/tree.cc.
Aha, I tried this way first and consider that maybe it should be something like types_match.
Thus, sent the v1 for this, will go bitmask_inv_cst_vector_p in v3.
Pan
-----Original Message-----
From: Tamar Christina <Tamar.Christina@arm.com>
Sent: Monday, May 20, 2024 7:20 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
Cc: juzhe.zhong@rivai.ai; kito.cheng@gmail.com; richard.guenther@gmail.com
Subject: RE: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
> -----Original Message-----
> From: pan2.li@intel.com <pan2.li@intel.com>
> Sent: Sunday, May 19, 2024 5:17 AM
> To: gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; kito.cheng@gmail.com; Tamar Christina
> <Tamar.Christina@arm.com>; richard.guenther@gmail.com; Pan Li
> <pan2.li@intel.com>
> Subject: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid
> code dup [NFC]
>
> From: Pan Li <pan2.li@intel.com>
>
> There are sorts of match pattern for SAT related cases, there will be
> some duplicated code to check the dest, op_0, op_1 are same tree types.
> Aka ternary tree type matches. Thus, extract one helper function to
> do this and avoid match code duplication.
>
> The below test suites are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 regression test.
>
> gcc/ChangeLog:
>
> * generic-match-head.cc (integer_types_ternary_match): New helper
> function to check tenary tree type matches or not.
> * gimple-match-head.cc (integer_types_ternary_match): Ditto but
> for match.
> * match.pd: Leverage above helper function to avoid code dup.
>
Nice cleanup!
This function isn't part of the machinery of match.pd and is instead part of a pattern.
For these things we usually put them in tree.h/tree.cc and declare them at the top
of match.pd in the "define_predicates" list.
This will also allow you to get rid of the code duplication. In addition such functions
which return a true/false we consider predicates, and name them ending with _p.
See e.g. bitmask_inv_cst_vector_p which is also defined in tree.h/tree.cc.
Cheers,
Tamar
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/generic-match-head.cc | 17 +++++++++++++++++
> gcc/gimple-match-head.cc | 17 +++++++++++++++++
> gcc/match.pd | 25 +++++--------------------
> 3 files changed, 39 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
> index 0d3f648fe8d..cdd48c7a5cc 100644
> --- a/gcc/generic-match-head.cc
> +++ b/gcc/generic-match-head.cc
> @@ -59,6 +59,23 @@ types_match (tree t1, tree t2)
> return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2);
> }
>
> +/* Routine to determine if the types T1, T2 and T3 are effectively
> + the same integer type for GENERIC. If T1, T2 or T3 is not a type,
> + the test applies to their TREE_TYPE. */
> +
> +static inline bool
> +integer_types_ternary_match (tree t1, tree t2, tree t3)
> +{
> + t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
> + t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
> + t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
> +
> + if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P
> (t3))
> + return false;
> +
> + return types_match (t1, t2) && types_match (t1, t3);
> +}
> +
> /* Return if T has a single use. For GENERIC, we assume this is
> always true. */
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index 5f8a1a1ad8e..91f2e56b8ef 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -79,6 +79,23 @@ types_match (tree t1, tree t2)
> return types_compatible_p (t1, t2);
> }
>
> +/* Routine to determine if the types T1, T2 and T3 are effectively
> + the same integer type for GIMPLE. If T1, T2 or T3 is not a type,
> + the test applies to their TREE_TYPE. */
> +
> +static inline bool
> +integer_types_ternary_match (tree t1, tree t2, tree t3)
> +{
> + t1 = TYPE_P (t1) ? t1 : TREE_TYPE (t1);
> + t2 = TYPE_P (t2) ? t2 : TREE_TYPE (t2);
> + t3 = TYPE_P (t3) ? t3 : TREE_TYPE (t3);
> +
> + if (!INTEGRAL_TYPE_P (t1) || !INTEGRAL_TYPE_P (t2) || !INTEGRAL_TYPE_P
> (t3))
> + return false;
> +
> + return types_match (t1, t2) && types_match (t1, t3);
> +}
> +
> /* Return if T has a single use. For GIMPLE, we also allow any
> non-SSA_NAME (ie constants) and zero uses to cope with uses
> that aren't linked up yet. */
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 0f9c34fa897..b291e34bbe4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3046,38 +3046,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* Unsigned Saturation Add */
> (match (usadd_left_part_1 @0 @1)
> (plus:c @0 @1)
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_left_part_2 @0 @1)
> (realpart (IFN_ADD_OVERFLOW:c @0 @1))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_right_part_1 @0 @1)
> (negate (convert (lt (plus:c @0 @1) @0)))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_right_part_1 @0 @1)
> (negate (convert (gt @0 (plus:c @0 @1))))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> (match (usadd_right_part_2 @0 @1)
> (negate (convert (ne (imagpart (IFN_ADD_OVERFLOW:c @0 @1))
> integer_zerop)))
> - (if (INTEGRAL_TYPE_P (type)
> - && TYPE_UNSIGNED (TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@0))
> - && types_match (type, TREE_TYPE (@1)))))
> + (if (TYPE_UNSIGNED (type) && integer_types_ternary_match (type, @0, @1))))
>
> /* We cannot merge or overload usadd_left_part_1 and usadd_left_part_2
> because the sub part of left_part_2 cannot work with right_part_1.
> --
> 2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-20 11:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-19 4:16 [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC] pan2.li
2024-05-19 4:24 ` Andrew Pinski
2024-05-20 11:00 ` Li, Pan2
2024-05-20 11:20 ` Tamar Christina
2024-05-20 11:48 ` Li, Pan2
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).