Richard Biener <richard.guent...@gmail.com> writes:
> 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
Yeah, it's unnecessary. The idea was to (try to) ensure that
gimple_simplify keeps all the microoptimisations that it had
previously. This includes open-coding do_valueize for SSA_NAMEs
and jumping straight to the right gimplify_resimplifyN routine
when the number of operands is already known.
(The two calls to gimple_extract<> produce different functions
that ought to get inlined into their single callers. A lot of the
jumps should then be threaded.)
I can drop all that if you don't think it's worth it though.
Just wanted to double-check first.
Thanks,
Richard
>> + - 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
>>
