| Message ID | 20240520105956.2826662-1-pan2.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC] | expand |
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 >
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 --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.