> +/* Analyze EXPR if it represents a series of simple operations performed on
> + a function parameter and return true if so. FBI, STMT, EXPR, INDEX_P and
> + AGGPOS have the same meaning like in unmodified_parm_or_parm_agg_item.
> + Type of the parameter or load from an aggregate via the parameter is
> + stored in *TYPE_P. Operations on the parameter are recorded to
> + PARAM_OPS_P if it is not NULL. */
> +
> +static bool
> +decompose_param_expr (struct ipa_func_body_info *fbi,
> + gimple *stmt, tree expr,
> + int *index_p, tree *type_p,
> + struct agg_position_info *aggpos,
> + expr_eval_ops *param_ops_p = NULL)
> +{
> + int op_limit = PARAM_VALUE (PARAM_IPA_MAX_PARAM_EXPR_OPS);
> + int op_count = 0;
> +
> + if (param_ops_p)
> + *param_ops_p = NULL;
> +
> + while (true)
> + {
> + expr_eval_op eval_op;
> + poly_int64 size;
> +
> + if (unmodified_parm_or_parm_agg_item (fbi, stmt, expr, index_p, &size,
> + aggpos))
> + {
> + tree type = TREE_TYPE (expr);
> +
> + /* Stop if found bit-field whose size does not equal any natural
> + integral type. */
> + if (maybe_ne (tree_to_poly_int64 (TYPE_SIZE (type)), size))
> + break;
Why is this needed?
> +
> + *type_p = type;
> + return true;
> + }
> +
> + if (TREE_CODE (expr) != SSA_NAME || SSA_NAME_IS_DEFAULT_DEF (expr))
> + break;
> +
> + if (!is_gimple_assign (stmt = SSA_NAME_DEF_STMT (expr)))
> + break;
> +
> + switch (gimple_assign_rhs_class (stmt))
> + {
> + case GIMPLE_SINGLE_RHS:
> + expr = gimple_assign_rhs1 (stmt);
> + continue;
> +
> + case GIMPLE_UNARY_RHS:
> + expr = gimple_assign_rhs1 (stmt);
> +
> + eval_op.val_is_rhs = false;
I find val_is_rhs to be confusing, lhs/rhs is usually used for the
gimple assignments. Here everything is rhs, but it depends whether
the val is operand0 or operand1. So I guess we could do val_is_op1.
> @@ -1183,10 +1301,8 @@ set_cond_stmt_execution_predicate (struct
> ipa_func_body_info *fbi,
> if (!is_gimple_ip_invariant (gimple_cond_rhs (last)))
> return;
> op = gimple_cond_lhs (last);
> - /* TODO: handle conditionals like
> - var = op0 < 4;
> - if (var != 0). */
> - if (unmodified_parm_or_parm_agg_item (fbi, last, op, &index, &size,
> &aggpos))
> +
> + if (decompose_param_expr (fbi, last, op, &index, &type, &aggpos,
> ¶m_ops))
> {
> code = gimple_cond_code (last);
> inverted_code = invert_tree_comparison (code, HONOR_NANS (op));
> @@ -1201,13 +1317,13 @@ set_cond_stmt_execution_predicate (struct
> ipa_func_body_info *fbi,
> if (this_code != ERROR_MARK)
> {
> predicate p
> - = add_condition (summary, index, size, &aggpos, this_code,
> - unshare_expr_without_location
> - (gimple_cond_rhs (last)));
> + = add_condition (summary, index, type, &aggpos, this_code,
> + gimple_cond_rhs (last), param_ops);
An why unsharing is no longer needed here?
It is importnat to avoid anything which reffers to function local blocks
to get into the global LTO stream.
> +/* Check whether two set of operations have same effects. */
> +static bool
> +expr_eval_ops_equal_p (expr_eval_ops ops1, expr_eval_ops ops2)
> +{
> + if (ops1)
> + {
> + if (!ops2 || ops1->length () != ops2->length ())
> + return false;
> +
> + for (unsigned i = 0; i < ops1->length (); i++)
> + {
> + expr_eval_op &op1 = (*ops1)[i];
> + expr_eval_op &op2 = (*ops2)[i];
> +
> + if (op1.code != op2.code
> + || op1.val_is_rhs != op2.val_is_rhs
> + || !vrp_operand_equal_p (op1.val, op2.val)
Why you need vrp_operand_equal_p instead of operand_equal_p here?
Overall the patch looks very reasonable. We may end up with bit more
general expressions (i.e. supporting arithmetics involving more than one
operand).
I see you also fixed a lot of typos in comments, please go head and
commit them separately as obvious.
Thank for all the work!
Honza
