Re: [PATCH][match-and-simplify] Hande side-effects in GENERIC

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Oct 22, 2014 at 04:20:09PM +0200, Richard Biener wrote:
> 2014-10-22  Richard Biener  <rguenther@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