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.guent...@gmail.com> 
Sent: Wednesday, May 22, 2024 9:04 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; 
tamar.christ...@arm.com; pins...@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...@intel.com> wrote:
>
> From: Pan Li <pan2...@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...@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
>

Reply via email to