diff mbox series

[v2] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]

Message ID 20240520105956.2826662-1-pan2.li@intel.com
State New
Headers
Series [v2] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Li, Pan2 May 20, 2024, 10:59 a.m. UTC 
  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(-)

Comments

Richard Biener May 22, 2024, 1:04 p.m. UTC | #1 
On Mon, May 20, 2024 at 1:00 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.

I think it's more useful to add an overload to types_match with three
arguments and then use

 (if (INTEGRAL_TYPE_P (type)
      && types_match (type, TREE_TYPE (@0), TREE_TYPE (@1))
...

Richard.

> 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..401b52e7573 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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  /* 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
>
Li, Pan2 May 22, 2024, 2:27 p.m. UTC | #2 
Thanks Richard for comments.

> I think it's more useful to add an overload to types_match with three
> arguments and then use

> (if (INTEGRAL_TYPE_P (type)
>       && types_match (type, TREE_TYPE (@0), TREE_TYPE (@1))

Sure thing, will try to add overloaded types_match here.

Pan

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: Wednesday, May 22, 2024 9:04 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; tamar.christina@arm.com; pinskia@gmail.com
Subject: Re: [PATCH v2] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]

On Mon, May 20, 2024 at 1:00 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.

I think it's more useful to add an overload to types_match with three
arguments and then use

 (if (INTEGRAL_TYPE_P (type)
      && types_match (type, TREE_TYPE (@0), TREE_TYPE (@1))
...

Richard.

> 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..401b52e7573 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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
>
>  /* 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
>
diff mbox series

Patch

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..401b52e7573 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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
 
 (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
 
 (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
 
 (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
 
 (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 (integer_types_ternary_match (type, @0, @1) && TYPE_UNSIGNED (type))))
 
 /* 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.