[2/5] gimple-match: Add a gimple_extract_op function

  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(-)
  

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
>
  

Richard Biener <richard.guenther@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
>>
  

On Tue, Nov 16, 2021 at 4:51 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@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.

Yes, I'd prefer the simpler and more understandable variant.

Richard.

>
> 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
> >>
  

Richard Biener <richard.guenther@gmail.com> writes:
> On Tue, Nov 16, 2021 at 4:51 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener <richard.guenther@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.
>
> Yes, I'd prefer the simpler and more understandable variant.

Fair :-)  How does this look?  I pulled in the extra conversions
you mentioned in the later review since they're useful for the
new gimple_simplify routine.

Tested as before.

Richard


gcc/
	* gimple-match.h (code_helper): Add functions for querying whether
	the code represents an internal_fn or a built_in_function.
	Provide explicit conversion operators for both cases.
	(gimple_extract_op): Declare.
	* gimple-match-head.c (gimple_extract): New function, extracted from...
	(gimple_simplify): ...here.
	(gimple_extract_op): New function.
---
 gcc/gimple-match-head.c | 218 ++++++++++++++++++++--------------------
 gcc/gimple-match.h      |  27 +++++
 2 files changed, 135 insertions(+), 110 deletions(-)

diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
index 9d88b2f8551..15053d1189e 100644
--- a/gcc/gimple-match-head.c
+++ b/gcc/gimple-match-head.c
@@ -890,12 +890,20 @@ 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, returning true on success.  Before recording
+   an operand, call:
 
-bool
-gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
-		 tree (*valueize)(tree), tree (*top_valueize)(tree))
+   - VALUEIZE_CONDITION for a COND_EXPR condition
+   - VALUEIZE_OP for every other top-level operand
+
+   Both routines take a tree argument and returns a tree.  */
+
+template<typename ValueizeOp, typename ValueizeCondition>
+inline bool
+gimple_extract (gimple *stmt, gimple_match_op *res_op,
+		ValueizeOp valueize_op,
+		ValueizeCondition valueize_condition)
 {
   switch (gimple_code (stmt))
     {
@@ -911,100 +919,50 @@ 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 true;
 	      }
 	    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 true;
 	      }
-	    else if (code == SSA_NAME
-		     && top_valueize)
+	    else if (code == SSA_NAME)
 	      {
 		tree op0 = gimple_assign_rhs1 (stmt);
-		tree valueized = top_valueize (op0);
-		if (!valueized || op0 == valueized)
-		  return false;
-		res_op->set_op (TREE_CODE (op0), type, valueized);
+		res_op->set_op (TREE_CODE (op0), type, valueize_op (op0));
 		return true;
 	      }
 	    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 true;
 	    }
 	  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 true;
 	    }
 	  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 true;
 	    }
 	  default:
 	    gcc_unreachable ();
@@ -1018,7 +976,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));
@@ -1028,7 +985,7 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
 	      if (!fn)
 		return false;
 
-	      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;
@@ -1044,47 +1001,17 @@ 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 true;
 	}
       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 true;
       }
 
     default:
@@ -1094,6 +1021,77 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
   return false;
 }
 
+/* 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);
+}
+
+/* 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);
+    };
+
+  if (!gimple_extract (stmt, res_op, valueize_op, valueize_condition))
+    return false;
+
+  if (res_op->code.is_internal_fn ())
+    {
+      internal_fn ifn = internal_fn (res_op->code);
+      if (try_conditional_simplification (ifn, res_op, seq, valueize))
+	return true;
+    }
+
+  if (!res_op->reverse
+      && res_op->num_ops
+      && res_op->resimplify (seq, valueize))
+    return true;
+
+  return valueized;
+}
 
 /* Helper for the autogenerated code, valueize OP.  */
 
diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h
index 2d4ea476076..8abeafebb00 100644
--- a/gcc/gimple-match.h
+++ b/gcc/gimple-match.h
@@ -33,13 +33,39 @@ public:
   code_helper (combined_fn fn) : rep (-(int) fn) {}
   operator tree_code () const { return (tree_code) rep; }
   operator combined_fn () const { return (combined_fn) -rep; }
+  explicit operator internal_fn () const;
+  explicit operator built_in_function () const;
   bool is_tree_code () const { return rep > 0; }
   bool is_fn_code () const { return rep < 0; }
+  bool is_internal_fn () const;
+  bool is_builtin_fn () const;
   int get_rep () const { return rep; }
 private:
   int rep;
 };
 
+inline code_helper::operator internal_fn () const
+{
+  return as_internal_fn (combined_fn (*this));
+}
+
+inline code_helper::operator built_in_function () const
+{
+  return as_builtin_fn (combined_fn (*this));
+}
+
+inline bool
+code_helper::is_internal_fn () const
+{
+  return is_fn_code () && internal_fn_p (combined_fn (*this));
+}
+
+inline bool
+code_helper::is_builtin_fn () const
+{
+  return is_fn_code () && builtin_fn_p (combined_fn (*this));
+}
+
 /* Represents the condition under which an operation should happen,
    and the value to use otherwise.  The condition applies elementwise
    (as for VEC_COND_EXPR) if the values are vectors.  */
@@ -333,6 +359,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 *,
  

On Thu, Nov 18, 2021 at 8:01 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Tue, Nov 16, 2021 at 4:51 PM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> Richard Biener <richard.guenther@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.
> >
> > Yes, I'd prefer the simpler and more understandable variant.
>
> Fair :-)  How does this look?  I pulled in the extra conversions
> you mentioned in the later review since they're useful for the
> new gimple_simplify routine.

OK.

Thanks,
Richard.

> Tested as before.
>
> Richard
>
>
> gcc/
>         * gimple-match.h (code_helper): Add functions for querying whether
>         the code represents an internal_fn or a built_in_function.
>         Provide explicit conversion operators for both cases.
>         (gimple_extract_op): Declare.
>         * gimple-match-head.c (gimple_extract): New function, extracted from...
>         (gimple_simplify): ...here.
>         (gimple_extract_op): New function.
> ---
>  gcc/gimple-match-head.c | 218 ++++++++++++++++++++--------------------
>  gcc/gimple-match.h      |  27 +++++
>  2 files changed, 135 insertions(+), 110 deletions(-)
>
> diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
> index 9d88b2f8551..15053d1189e 100644
> --- a/gcc/gimple-match-head.c
> +++ b/gcc/gimple-match-head.c
> @@ -890,12 +890,20 @@ 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, returning true on success.  Before recording
> +   an operand, call:
>
> -bool
> -gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
> -                tree (*valueize)(tree), tree (*top_valueize)(tree))
> +   - VALUEIZE_CONDITION for a COND_EXPR condition
> +   - VALUEIZE_OP for every other top-level operand
> +
> +   Both routines take a tree argument and returns a tree.  */
> +
> +template<typename ValueizeOp, typename ValueizeCondition>
> +inline bool
> +gimple_extract (gimple *stmt, gimple_match_op *res_op,
> +               ValueizeOp valueize_op,
> +               ValueizeCondition valueize_condition)
>  {
>    switch (gimple_code (stmt))
>      {
> @@ -911,100 +919,50 @@ 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 true;
>               }
>             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 true;
>               }
> -           else if (code == SSA_NAME
> -                    && top_valueize)
> +           else if (code == SSA_NAME)
>               {
>                 tree op0 = gimple_assign_rhs1 (stmt);
> -               tree valueized = top_valueize (op0);
> -               if (!valueized || op0 == valueized)
> -                 return false;
> -               res_op->set_op (TREE_CODE (op0), type, valueized);
> +               res_op->set_op (TREE_CODE (op0), type, valueize_op (op0));
>                 return true;
>               }
>             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 true;
>             }
>           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 true;
>             }
>           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 true;
>             }
>           default:
>             gcc_unreachable ();
> @@ -1018,7 +976,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));
> @@ -1028,7 +985,7 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
>               if (!fn)
>                 return false;
>
> -             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;
> @@ -1044,47 +1001,17 @@ 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 true;
>         }
>        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 true;
>        }
>
>      default:
> @@ -1094,6 +1021,77 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq,
>    return false;
>  }
>
> +/* 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);
> +}
> +
> +/* 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);
> +    };
> +
> +  if (!gimple_extract (stmt, res_op, valueize_op, valueize_condition))
> +    return false;
> +
> +  if (res_op->code.is_internal_fn ())
> +    {
> +      internal_fn ifn = internal_fn (res_op->code);
> +      if (try_conditional_simplification (ifn, res_op, seq, valueize))
> +       return true;
> +    }
> +
> +  if (!res_op->reverse
> +      && res_op->num_ops
> +      && res_op->resimplify (seq, valueize))
> +    return true;
> +
> +  return valueized;
> +}
>
>  /* Helper for the autogenerated code, valueize OP.  */
>
> diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h
> index 2d4ea476076..8abeafebb00 100644
> --- a/gcc/gimple-match.h
> +++ b/gcc/gimple-match.h
> @@ -33,13 +33,39 @@ public:
>    code_helper (combined_fn fn) : rep (-(int) fn) {}
>    operator tree_code () const { return (tree_code) rep; }
>    operator combined_fn () const { return (combined_fn) -rep; }
> +  explicit operator internal_fn () const;
> +  explicit operator built_in_function () const;
>    bool is_tree_code () const { return rep > 0; }
>    bool is_fn_code () const { return rep < 0; }
> +  bool is_internal_fn () const;
> +  bool is_builtin_fn () const;
>    int get_rep () const { return rep; }
>  private:
>    int rep;
>  };
>
> +inline code_helper::operator internal_fn () const
> +{
> +  return as_internal_fn (combined_fn (*this));
> +}
> +
> +inline code_helper::operator built_in_function () const
> +{
> +  return as_builtin_fn (combined_fn (*this));
> +}
> +
> +inline bool
> +code_helper::is_internal_fn () const
> +{
> +  return is_fn_code () && internal_fn_p (combined_fn (*this));
> +}
> +
> +inline bool
> +code_helper::is_builtin_fn () const
> +{
> +  return is_fn_code () && builtin_fn_p (combined_fn (*this));
> +}
> +
>  /* Represents the condition under which an operation should happen,
>     and the value to use otherwise.  The condition applies elementwise
>     (as for VEC_COND_EXPR) if the values are vectors.  */
> @@ -333,6 +359,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
>