From: "Li, Pan2" <pan2.li@intel.com>
To: Tamar Christina <Tamar.Christina@arm.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>,
"kito.cheng@gmail.com" <kito.cheng@gmail.com>,
"richard.guenther@gmail.com" <richard.guenther@gmail.com>
Subject: RE: [PATCH v1] Match: Extract integer_types_ternary_match helper to avoid code dup [NFC]
Date: Mon, 20 May 2024 11:48:21 +0000 [thread overview]
Message-ID: <MW5PR11MB5908E5FD8D7FB54C780656D7A9E92@MW5PR11MB5908.namprd11.prod.outlook.com> (raw)
In-Reply-To: <VI1PR08MB532542618C1D1A8238E86846FFE92@VI1PR08MB5325.eurprd08.prod.outlook.com>
> 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
prev parent reply other threads:[~2024-05-20 11:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-19 4:16 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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MW5PR11MB5908E5FD8D7FB54C780656D7A9E92@MW5PR11MB5908.namprd11.prod.outlook.com \
--to=pan2.li@intel.com \
--cc=Tamar.Christina@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=juzhe.zhong@rivai.ai \
--cc=kito.cheng@gmail.com \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).