From: Richard Biener <richard.guenther@gmail.com>
To: Richard Sandiford <richard.sandiford@arm.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 2/5] gimple-match: Add a gimple_extract_op function
Date: Thu, 11 Nov 2021 11:49:26 +0100 [thread overview]
Message-ID: <CAFiYyc0cPurVZ4MdnmLywaXHv6iGd_hAAmHaS4ZhA8coEqn=zA@mail.gmail.com> (raw)
In-Reply-To: <mpt8rxww56o.fsf@arm.com>
On Wed, Nov 10, 2021 at 1:46 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> code_helper and gimple_match_op seem like generally useful ways
> of summing up a gimple_assign or gimple_call (or gimple_cond).
> This patch adds a gimple_extract_op function that can be used
> for that.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>
> Richard
>
>
> gcc/
> * gimple-match.h (gimple_extract_op): Declare.
> * gimple-match.c (gimple_extract): New function, extracted from...
> (gimple_simplify): ...here.
> (gimple_extract_op): New function.
> ---
> gcc/gimple-match-head.c | 261 +++++++++++++++++++++++-----------------
> gcc/gimple-match.h | 1 +
> 2 files changed, 149 insertions(+), 113 deletions(-)
>
> diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
> index 9d88b2f8551..4c6e0883ba4 100644
> --- a/gcc/gimple-match-head.c
> +++ b/gcc/gimple-match-head.c
> @@ -890,12 +890,29 @@ try_conditional_simplification (internal_fn ifn, gimple_match_op *res_op,
> return true;
> }
>
> -/* The main STMT based simplification entry. It is used by the fold_stmt
> - and the fold_stmt_to_constant APIs. */
> +/* Common subroutine of gimple_extract_op and gimple_simplify. Try to
> + describe STMT in RES_OP. Return:
>
> -bool
> -gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> - tree (*valueize)(tree), tree (*top_valueize)(tree))
> + - -1 if extraction failed
> + - otherwise, 0 if no simplification should take place
> + - otherwise, the number of operands for a GIMPLE_ASSIGN or GIMPLE_COND
> + - otherwise, -2 for a GIMPLE_CALL
> +
> + Before recording an operand, call:
> +
> + - VALUEIZE_CONDITION for a COND_EXPR condition
> + - VALUEIZE_NAME if the rhs of a GIMPLE_ASSIGN is an SSA_NAME
I think at least VALUEIZE_NAME is unnecessary, see below
> + - VALUEIZE_OP for every other top-level operand
> +
> + Each routine takes a tree argument and returns a tree. */
> +
> +template<typename ValueizeOp, typename ValueizeCondition,
> + typename ValueizeName>
> +inline int
> +gimple_extract (gimple *stmt, gimple_match_op *res_op,
> + ValueizeOp valueize_op,
> + ValueizeCondition valueize_condition,
> + ValueizeName valueize_name)
> {
> switch (gimple_code (stmt))
> {
> @@ -911,100 +928,53 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> || code == VIEW_CONVERT_EXPR)
> {
> tree op0 = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
> - bool valueized = false;
> - op0 = do_valueize (op0, top_valueize, valueized);
> - res_op->set_op (code, type, op0);
> - return (gimple_resimplify1 (seq, res_op, valueize)
> - || valueized);
> + res_op->set_op (code, type, valueize_op (op0));
> + return 1;
> }
> else if (code == BIT_FIELD_REF)
> {
> tree rhs1 = gimple_assign_rhs1 (stmt);
> - tree op0 = TREE_OPERAND (rhs1, 0);
> - bool valueized = false;
> - op0 = do_valueize (op0, top_valueize, valueized);
> + tree op0 = valueize_op (TREE_OPERAND (rhs1, 0));
> res_op->set_op (code, type, op0,
> TREE_OPERAND (rhs1, 1),
> TREE_OPERAND (rhs1, 2),
> REF_REVERSE_STORAGE_ORDER (rhs1));
> - if (res_op->reverse)
> - return valueized;
> - return (gimple_resimplify3 (seq, res_op, valueize)
> - || valueized);
> + return res_op->reverse ? 0 : 3;
> }
> - else if (code == SSA_NAME
> - && top_valueize)
> + else if (code == SSA_NAME)
> {
> tree op0 = gimple_assign_rhs1 (stmt);
> - tree valueized = top_valueize (op0);
> + tree valueized = valueize_name (op0);
> if (!valueized || op0 == valueized)
> - return false;
> + return -1;
> res_op->set_op (TREE_CODE (op0), type, valueized);
> - return true;
> + return 0;
So the old code in an obfuscated way just knowed nothing simplifies
on the plain not valueized name but returned true when valueization
changed the stmt. So I'd expect
tree valueized = valueize_op (op0);
res_op->set_op (TREE_CODE (op0), type, valueized);
return 0;
here and the gimple_simplify caller returning 'valueized'. I think
that the old code treated a NULL top_valueize () as "fail" is just
premature optimization without any effect.
> }
> break;
> case GIMPLE_UNARY_RHS:
> {
> tree rhs1 = gimple_assign_rhs1 (stmt);
> - bool valueized = false;
> - rhs1 = do_valueize (rhs1, top_valueize, valueized);
> - res_op->set_op (code, type, rhs1);
> - return (gimple_resimplify1 (seq, res_op, valueize)
> - || valueized);
> + res_op->set_op (code, type, valueize_op (rhs1));
> + return 1;
> }
> case GIMPLE_BINARY_RHS:
> {
> - tree rhs1 = gimple_assign_rhs1 (stmt);
> - tree rhs2 = gimple_assign_rhs2 (stmt);
> - bool valueized = false;
> - rhs1 = do_valueize (rhs1, top_valueize, valueized);
> - rhs2 = do_valueize (rhs2, top_valueize, valueized);
> + tree rhs1 = valueize_op (gimple_assign_rhs1 (stmt));
> + tree rhs2 = valueize_op (gimple_assign_rhs2 (stmt));
> res_op->set_op (code, type, rhs1, rhs2);
> - return (gimple_resimplify2 (seq, res_op, valueize)
> - || valueized);
> + return 2;
> }
> case GIMPLE_TERNARY_RHS:
> {
> - bool valueized = false;
> tree rhs1 = gimple_assign_rhs1 (stmt);
> - /* If this is a COND_EXPR first try to simplify an
> - embedded GENERIC condition. */
> - if (code == COND_EXPR)
> - {
> - if (COMPARISON_CLASS_P (rhs1))
> - {
> - tree lhs = TREE_OPERAND (rhs1, 0);
> - tree rhs = TREE_OPERAND (rhs1, 1);
> - lhs = do_valueize (lhs, top_valueize, valueized);
> - rhs = do_valueize (rhs, top_valueize, valueized);
> - gimple_match_op res_op2 (res_op->cond, TREE_CODE (rhs1),
> - TREE_TYPE (rhs1), lhs, rhs);
> - if ((gimple_resimplify2 (seq, &res_op2, valueize)
> - || valueized)
> - && res_op2.code.is_tree_code ())
> - {
> - valueized = true;
> - if (TREE_CODE_CLASS ((enum tree_code) res_op2.code)
> - == tcc_comparison)
> - rhs1 = build2 (res_op2.code, TREE_TYPE (rhs1),
> - res_op2.ops[0], res_op2.ops[1]);
> - else if (res_op2.code == SSA_NAME
> - || res_op2.code == INTEGER_CST
> - || res_op2.code == VECTOR_CST)
> - rhs1 = res_op2.ops[0];
> - else
> - valueized = false;
> - }
> - }
> - }
> - tree rhs2 = gimple_assign_rhs2 (stmt);
> - tree rhs3 = gimple_assign_rhs3 (stmt);
> - rhs1 = do_valueize (rhs1, top_valueize, valueized);
> - rhs2 = do_valueize (rhs2, top_valueize, valueized);
> - rhs3 = do_valueize (rhs3, top_valueize, valueized);
> + if (code == COND_EXPR && COMPARISON_CLASS_P (rhs1))
> + rhs1 = valueize_condition (rhs1);
> + else
> + rhs1 = valueize_op (rhs1);
> + tree rhs2 = valueize_op (gimple_assign_rhs2 (stmt));
> + tree rhs3 = valueize_op (gimple_assign_rhs3 (stmt));
> res_op->set_op (code, type, rhs1, rhs2, rhs3);
> - return (gimple_resimplify3 (seq, res_op, valueize)
> - || valueized);
> + return 3;
> }
> default:
> gcc_unreachable ();
> @@ -1018,7 +988,6 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> && gimple_call_num_args (stmt) >= 1
> && gimple_call_num_args (stmt) <= 5)
> {
> - bool valueized = false;
> combined_fn cfn;
> if (gimple_call_internal_p (stmt))
> cfn = as_combined_fn (gimple_call_internal_fn (stmt));
> @@ -1026,17 +995,17 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> {
> tree fn = gimple_call_fn (stmt);
> if (!fn)
> - return false;
> + return -1;
>
> - fn = do_valueize (fn, top_valueize, valueized);
> + fn = valueize_op (fn);
> if (TREE_CODE (fn) != ADDR_EXPR
> || TREE_CODE (TREE_OPERAND (fn, 0)) != FUNCTION_DECL)
> - return false;
> + return -1;
>
> tree decl = TREE_OPERAND (fn, 0);
> if (DECL_BUILT_IN_CLASS (decl) != BUILT_IN_NORMAL
> || !gimple_builtin_call_types_compatible_p (stmt, decl))
> - return false;
> + return -1;
>
> cfn = as_combined_fn (DECL_FUNCTION_CODE (decl));
> }
> @@ -1044,56 +1013,122 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> unsigned int num_args = gimple_call_num_args (stmt);
> res_op->set_op (cfn, TREE_TYPE (gimple_call_lhs (stmt)), num_args);
> for (unsigned i = 0; i < num_args; ++i)
> - {
> - tree arg = gimple_call_arg (stmt, i);
> - res_op->ops[i] = do_valueize (arg, top_valueize, valueized);
> - }
> - if (internal_fn_p (cfn)
> - && try_conditional_simplification (as_internal_fn (cfn),
> - res_op, seq, valueize))
> - return true;
> - switch (num_args)
> - {
> - case 1:
> - return (gimple_resimplify1 (seq, res_op, valueize)
> - || valueized);
> - case 2:
> - return (gimple_resimplify2 (seq, res_op, valueize)
> - || valueized);
> - case 3:
> - return (gimple_resimplify3 (seq, res_op, valueize)
> - || valueized);
> - case 4:
> - return (gimple_resimplify4 (seq, res_op, valueize)
> - || valueized);
> - case 5:
> - return (gimple_resimplify5 (seq, res_op, valueize)
> - || valueized);
> - default:
> - gcc_unreachable ();
> - }
> + res_op->ops[i] = valueize_op (gimple_call_arg (stmt, i));
> + return -2;
> }
> break;
>
> case GIMPLE_COND:
> {
> - tree lhs = gimple_cond_lhs (stmt);
> - tree rhs = gimple_cond_rhs (stmt);
> - bool valueized = false;
> - lhs = do_valueize (lhs, top_valueize, valueized);
> - rhs = do_valueize (rhs, top_valueize, valueized);
> + tree lhs = valueize_op (gimple_cond_lhs (stmt));
> + tree rhs = valueize_op (gimple_cond_rhs (stmt));
> res_op->set_op (gimple_cond_code (stmt), boolean_type_node, lhs, rhs);
> - return (gimple_resimplify2 (seq, res_op, valueize)
> - || valueized);
> + return 2;
> }
>
> default:
> break;
> }
>
> - return false;
> + return -1;
> }
>
> +/* Try to describe STMT in RES_OP, returning true on success.
> + For GIMPLE_CONDs, describe the condition that is being tested.
> + For GIMPLE_ASSIGNs, describe the rhs of the assignment.
> + For GIMPLE_CALLs, describe the call. */
> +
> +bool
> +gimple_extract_op (gimple *stmt, gimple_match_op *res_op)
> +{
> + auto nop = [](tree op) { return op; };
> + return gimple_extract (stmt, res_op, nop, nop, nop) != -1;
> +}
> +
> +/* The main STMT based simplification entry. It is used by the fold_stmt
> + and the fold_stmt_to_constant APIs. */
> +
> +bool
> +gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> + tree (*valueize)(tree), tree (*top_valueize)(tree))
> +{
> + bool valueized = false;
> + auto valueize_op = [&](tree op)
> + {
> + return do_valueize (op, top_valueize, valueized);
> + };
> + auto valueize_condition = [&](tree op) -> tree
> + {
> + bool cond_valueized = false;
> + tree lhs = do_valueize (TREE_OPERAND (op, 0), top_valueize,
> + cond_valueized);
> + tree rhs = do_valueize (TREE_OPERAND (op, 1), top_valueize,
> + cond_valueized);
> + gimple_match_op res_op2 (res_op->cond, TREE_CODE (op),
> + TREE_TYPE (op), lhs, rhs);
> + if ((gimple_resimplify2 (seq, &res_op2, valueize)
> + || cond_valueized)
> + && res_op2.code.is_tree_code ())
> + {
> + if (TREE_CODE_CLASS ((tree_code) res_op2.code) == tcc_comparison)
> + {
> + valueized = true;
> + return build2 (res_op2.code, TREE_TYPE (op),
> + res_op2.ops[0], res_op2.ops[1]);
> + }
> + else if (res_op2.code == SSA_NAME
> + || res_op2.code == INTEGER_CST
> + || res_op2.code == VECTOR_CST)
> + {
> + valueized = true;
> + return res_op2.ops[0];
> + }
> + }
> + return valueize_op (op);
> + };
> + auto valueize_name = [&](tree op)
> + {
> + return top_valueize ? top_valueize (op) : op;
> + };
> +
> + int res = gimple_extract (stmt, res_op, valueize_op, valueize_condition,
> + valueize_name);
> + if (res == -1)
> + return false;
> +
> + if (res == -2)
> + {
> + combined_fn cfn = combined_fn (res_op->code);
> + if (internal_fn_p (cfn)
> + && try_conditional_simplification (as_internal_fn (cfn),
> + res_op, seq, valueize))
> + return true;
> + res = res_op->num_ops;
> + }
> +
> + switch (res)
> + {
> + case 0:
> + return valueized;
> + case 1:
> + return (gimple_resimplify1 (seq, res_op, valueize)
> + || valueized);
> + case 2:
> + return (gimple_resimplify2 (seq, res_op, valueize)
> + || valueized);
> + case 3:
> + return (gimple_resimplify3 (seq, res_op, valueize)
> + || valueized);
> + case 4:
> + return (gimple_resimplify4 (seq, res_op, valueize)
> + || valueized);
> + case 5:
> + return (gimple_resimplify5 (seq, res_op, valueize)
> + || valueized);
can't you use just res.resimplify (seq, valueize) || valueized instead of the
switch here when res is not zero?
Otherwise looks OK.
Thanks,
Richard.
> + default:
> + gcc_unreachable ();
> + }
> +}
>
> /* Helper for the autogenerated code, valueize OP. */
>
> diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h
> index 2d4ea476076..15a0f584db7 100644
> --- a/gcc/gimple-match.h
> +++ b/gcc/gimple-match.h
> @@ -333,6 +333,7 @@ gimple_simplified_result_is_gimple_val (const gimple_match_op *op)
>
> extern tree (*mprts_hook) (gimple_match_op *);
>
> +bool gimple_extract_op (gimple *, gimple_match_op *);
> bool gimple_simplify (gimple *, gimple_match_op *, gimple_seq *,
> tree (*)(tree), tree (*)(tree));
> tree maybe_push_res_to_seq (gimple_match_op *, gimple_seq *,
> --
> 2.25.1
>
next prev parent reply other threads:[~2021-11-11 10:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-10 12:44 Richard Sandiford
2021-11-11 10:49 ` Richard Biener [this message]
2021-11-16 15:51 ` Richard Sandiford
2021-11-18 14:27 ` Richard Biener
2021-11-18 19:01 ` Richard Sandiford
2021-11-19 9:13 ` Richard Biener
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='CAFiYyc0cPurVZ4MdnmLywaXHv6iGd_hAAmHaS4ZhA8coEqn=zA@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.sandiford@arm.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).