On Wed, Oct 22, 2014 at 04:20:09PM +0200, Richard Biener wrote:
> 2014-10-22 Richard Biener <rguent...@suse.de>
>
> * genmatch.c (count_captures): New function.
> (dt_simplify::gen): Handle preserving side-effects for
> GENERIC code generation.
> (decision_tree::gen_generic): Do not reject operands
> with TREE_SIDE_EFFECTS.
>
> Index: gcc/genmatch.c
> ===================================================================
> --- gcc/genmatch.c (revision 216549)
> +++ gcc/genmatch.c (working copy)
> @@ -1861,6 +1861,46 @@ dt_operand::gen (FILE *f, bool gimple)
> fprintf (f, "}\n");
> }
>
> +/* Count how many times captures are substituted in O and put the
> + result in the array of counters CPT. Return false if the
> + counting isn't precise. */
> +
> +static bool
> +count_captures (operand *o, int *cpt)
> +{
> + if (capture *c = dyn_cast <capture *> (o))
> + {
> + /* Give up for non-leafs (CSEs). */
> + if (c->what)
> + return false;
> + cpt[c->where]++;
> + }
> + else if (expr *e = dyn_cast <expr *> (o))
> + {
> + /* Give up for conditionally executed operands. */
> + if (*e->operation == COND_EXPR
> + || *e->operation == TRUTH_ANDIF_EXPR
> + || *e->operation == TRUTH_ORIF_EXPR)
> + return false;
As discussed on IRC, the problem is only captures inside of 2nd and 3rd
operand of COND_EXPR or 2nd operand of TRUTH_*IF_EXPR if the
first operand of those isn't constant (if it is constant, then depending
on the zero/nonzero value it should be either counted or ignored).
Captures in first operands of those are not a problem.
> @@ -1983,10 +2042,21 @@ dt_simplify::gen (FILE *f, bool gimple)
> }
> else /* GENERIC */
> {
> + bool is_predicate = false;
> if (result->type == operand::OP_EXPR)
> {
> expr *e = as_a <expr *> (result);
> - bool is_predicate = is_a <predicate_id *> (e->operation);
> + is_predicate = is_a <predicate_id *> (e->operation);
> + /* Search for captures used multiple times in the result expression
> + and dependent on TREE_SIDE_EFFECTS emit a SAVE_EXPR. */
> + if (!is_predicate && cpt[0] != -1)
> + for (int i = 0; i < s->capture_max + 1; ++i)
> + {
> + if (cpt[i] > 1)
> + fprintf (f, " if (TREE_SIDE_EFFECTS (captures[%d]))\n"
> + " captures[%d] = save_expr (captures[%d]);\n",
> + i, i, i);
> + }
For SAVE_EXPRs this will not do anything, which is right.
> + if (!is_predicate)
> + {
> + /* Search for captures not used in the result expression and dependent
> + on TREE_SIDE_EFFECTS emit omit_one_operand. */
> + if (cpt[0] != -1)
> + for (int i = 0; i < s->capture_max + 1; ++i)
> + {
> + if (cpt[i] == 0)
> + fprintf (f, " if (TREE_SIDE_EFFECTS (captures[%d]))\n"
> + " res = build2_loc (loc, COMPOUND_EXPR, "
> + "TREE_TYPE (captures[%d]), "
> + "fold_ignored_result (captures[%d]), res);\n",
> + i, i, i);
> + }
> + fprintf (f, " return res;\n");
> + }
For this case if captures[%d] happens to be a SAVE_EXPR, you
could perhaps avoid the COMPOUND_EXPR addition if the SAVE_EXPR is already
present somewhere else in the res (in unconditional context). Not sure
if it is worth the check (pretty much it would want a function which would
only add the COMPOUND_EXPR if the capture is not SAVE_EXPR or not found in
the expression).
Also, don't you want TREE_NO_WARNING on the COMPOUND_EXPR to avoid
-Wunused-value warnings (at least temporarily until C++ FE is fixed not to
fold everything early)?
And, perhaps it is better to queue all side-effects in lhs of toplevel
COMPOUND_EXPR, rather than having a chain of COMPOUND_EXPRs on the rhs?
I.e. first queue all side-effects into some temporary and if that temporary
is non-NULL, just add a single COMPOUND_EXPR with res as rhs and that
temporary as lhs.
Do we want to special case COMPOUND_EXPRs in further optimizations, or will
it be handled automatically (I mean, if trying to simplify
(something, expr1) op expr2
where expr1 op expr2 simplifies into something into
(something, result_of_simplification (expr1 op expr2)).
Jakub
