diff mbox

[match-and-simplify] Hande side-effects in GENERIC

Message ID alpine.LSU.2.11.1410221609380.9891@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Oct. 22, 2014, 2:20 p.m. UTC
The following auto-handles preserving of side-effects properly
for GENERIC simplification instead of simply rejecting operands
with side-effects.  Cases we cannot handle correctly are still
handled that way.

For example for

/* (x | CST1) & CST2 -> (x & CST2) | (CST1 & CST2) */
(simplify
  (bit_and (bit_ior @0 CONSTANT_CLASS_P@1) CONSTANT_CLASS_P@2)
  (bit_ior (bit_and @0 @2) (bit_and @1 @2)))

we generate

captures[0] = o20;
captures[1] = o21;
captures[2] = op1;
if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "Applying 
pattern match-bitwise.pd:59, %s:%d\n", __FILE__, __LINE__);
  if (TREE_SIDE_EFFECTS (captures[2]))
    captures[2] = save_expr (captures[2]);
...
  res = fold_build2_loc (loc, BIT_IOR_EXPR, type, res_op0, res_op1);
  return res;

of course somewhat pointless for CONSTANT_CLASS_P @2 (but well...).

For example for

(simplify
  (minus @0 @0)
  (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (TYPE_MODE (type)))
   { build_zero_cst (type); }))

we generate

captures[0] = op0;
/* #line 48 "/space/rguenther/src/svn/match-and-simplify/gcc/match.pd" */
if (!FLOAT_TYPE_P (type) || !HONOR_NANS (TYPE_MODE (type)))
  {
    if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, 
"Applying pattern match.pd:49, %s:%d\n", __FILE__, __LINE__);
    tree res;
    res =  build_zero_cst (type);
    if (TREE_SIDE_EFFECTS (captures[0]))
      res = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (captures[0]), 
fold_ignored_result (captures[0]), res);
    return res;
  }

I chose to open-code omit_N_operands.

For example for

(simplify
  (cond (bit_not @0) @1 @2)
  (cond @0 @2 @1))

we generate

captures[0] = o20;
captures[1] = op1;
captures[2] = op2;
if (TREE_SIDE_EFFECTS (op0) || TREE_SIDE_EFFECTS (op1) || 
TREE_SIDE_EFFECTS (op2)) return NULL_TREE;
if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "Applying 
pattern match.pd:207, %s:%d\n", __FILE__, __LINE__);
tree res_op0;
res_op0 = captures[0];
tree res_op1;
res_op1 = captures[2];
tree res_op2;
res_op2 = captures[1];
tree res;
res = fold_build3_loc (loc, COND_EXPR, type, res_op0, res_op1, res_op2);
  return res;

thus give up if any operand of the outer expression has side effects.

We currently give up for conditionally executed side-effects and
for C expressions in transforms that mention any captures (we
don't understand if they constitute a substitution into the
result).  I also give up if captured expressions are substituted
(they would make captures inside that expression used) and if
the replacement contains CSEs.  We could fix the latter two cases
if required.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

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.

Comments

Richard Biener Oct. 22, 2014, 2:35 p.m. UTC | #1
On Wed, 22 Oct 2014, Richard Biener wrote:

> 
> The following auto-handles preserving of side-effects properly
> for GENERIC simplification instead of simply rejecting operands
> with side-effects.  Cases we cannot handle correctly are still
> handled that way.
> 
> For example for
> 
> /* (x | CST1) & CST2 -> (x & CST2) | (CST1 & CST2) */
> (simplify
>   (bit_and (bit_ior @0 CONSTANT_CLASS_P@1) CONSTANT_CLASS_P@2)
>   (bit_ior (bit_and @0 @2) (bit_and @1 @2)))
> 
> we generate
> 
> captures[0] = o20;
> captures[1] = o21;
> captures[2] = op1;
> if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "Applying 
> pattern match-bitwise.pd:59, %s:%d\n", __FILE__, __LINE__);
>   if (TREE_SIDE_EFFECTS (captures[2]))
>     captures[2] = save_expr (captures[2]);
> ...
>   res = fold_build2_loc (loc, BIT_IOR_EXPR, type, res_op0, res_op1);
>   return res;
> 
> of course somewhat pointless for CONSTANT_CLASS_P @2 (but well...).
> 
> For example for
> 
> (simplify
>   (minus @0 @0)
>   (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (TYPE_MODE (type)))
>    { build_zero_cst (type); }))
> 
> we generate
> 
> captures[0] = op0;
> /* #line 48 "/space/rguenther/src/svn/match-and-simplify/gcc/match.pd" */
> if (!FLOAT_TYPE_P (type) || !HONOR_NANS (TYPE_MODE (type)))
>   {
>     if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, 
> "Applying pattern match.pd:49, %s:%d\n", __FILE__, __LINE__);
>     tree res;
>     res =  build_zero_cst (type);
>     if (TREE_SIDE_EFFECTS (captures[0]))
>       res = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (captures[0]), 
> fold_ignored_result (captures[0]), res);

Just noticed wrong type used here, fixed to do

 res = build2_loc (loc, COMPOUND_EXPR, type, fold_ignored_result (captures[0]), res);

Richard.

>     return res;
>   }
> 
> I chose to open-code omit_N_operands.
> 
> For example for
> 
> (simplify
>   (cond (bit_not @0) @1 @2)
>   (cond @0 @2 @1))
> 
> we generate
> 
> captures[0] = o20;
> captures[1] = op1;
> captures[2] = op2;
> if (TREE_SIDE_EFFECTS (op0) || TREE_SIDE_EFFECTS (op1) || 
> TREE_SIDE_EFFECTS (op2)) return NULL_TREE;
> if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "Applying 
> pattern match.pd:207, %s:%d\n", __FILE__, __LINE__);
> tree res_op0;
> res_op0 = captures[0];
> tree res_op1;
> res_op1 = captures[2];
> tree res_op2;
> res_op2 = captures[1];
> tree res;
> res = fold_build3_loc (loc, COND_EXPR, type, res_op0, res_op1, res_op2);
>   return res;
> 
> thus give up if any operand of the outer expression has side effects.
> 
> We currently give up for conditionally executed side-effects and
> for C expressions in transforms that mention any captures (we
> don't understand if they constitute a substitution into the
> result).  I also give up if captured expressions are substituted
> (they would make captures inside that expression used) and if
> the replacement contains CSEs.  We could fix the latter two cases
> if required.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> Richard.
> 
> 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;
> +      for (unsigned i = 0; i < e->ops.length (); ++i)
> +	if (!count_captures (e->ops[i], cpt))
> +	  return false;
> +    }
> +  else if (c_expr *e = dyn_cast <c_expr *> (o))
> +    {
> +      /* Give up for C exprs mentioning captures.  */
> +      for (unsigned i = 0; i < e->code.length (); ++i)
> +	if (e->code[i].type == CPP_ATSIGN
> +	    && (e->code[i+1].type == CPP_NUMBER
> +		|| e->code[i+1].type == CPP_NAME)
> +	    && !(e->code[i+1].flags & PREV_WHITE))
> +	  return false;
> +    }
> +  else
> +    gcc_unreachable ();
> +  return true;
> +}
> +
>  /* Generate code for the '(if ...)', '(with ..)' and actual transform
>     step of a '(simplify ...)' or '(match ...)'.  This handles everything
>     that is not part of the decision tree (simplify->match).  */
> @@ -1929,6 +1969,25 @@ dt_simplify::gen (FILE *f, bool gimple)
>        n_braces++;
>      }
>  
> +  /* For GENERIC we have to take care of wrapping multiple-used
> +     expressions with side-effects in save_expr and preserve side-effects
> +     of expressions with omit_one_operand.  Try to compute the number
> +     of times an expression is substituted.  */
> +  int *cpt = XALLOCAVEC (int, s->capture_max + 1);
> +  memset (cpt, 0, sizeof (int) * (s->capture_max + 1));
> +  if (!gimple
> +      && s->result
> +      && !count_captures (s->result, cpt))
> +    {
> +      /* Oops.  We could not get an exact count - give up and simply
> +         reject the pattern if any input operand had side-effects.  */
> +      fprintf (f, "if (TREE_SIDE_EFFECTS (op0)");
> +      for (unsigned i = 1; i < as_a <expr *> (s->match)->ops.length (); ++i)
> +	fprintf (f, " || TREE_SIDE_EFFECTS (op%u)", i);
> +      fprintf (f, ") return NULL_TREE;\n");
> +      cpt[0] = -1;
> +    }
> +
>    fprintf (f, "if (dump_file && (dump_flags & TDF_DETAILS)) "
>  	   "fprintf (dump_file, \"Applying pattern ");
>    output_line_directive (f, s->result_location, true);
> @@ -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 (unsigned j = 0; j < e->ops.length (); ++j)
>  	    {
>  	      char dest[32];
> @@ -2004,22 +2074,23 @@ dt_simplify::gen (FILE *f, bool gimple)
>  				    ? NULL : "TREE_TYPE (res_op0)");
>  	      e->ops[j]->gen_transform (f, dest, false, 1, optype, indexes);
>  	    }
> -	  if (is_a <predicate_id *> (e->operation))
> +	  if (is_predicate)
>  	    fprintf (f, "return true;\n");
>  	  else
>  	    {
> +	      fprintf (f, "  tree res;\n");
>  	      /* Re-fold the toplevel result.  Use non_lvalue to
>  	         build NON_LVALUE_EXPRs so they get properly
>  		 ignored when in GIMPLE form.  */
>  	      if (*e->operation == NON_LVALUE_EXPR)
> -		fprintf (f, "  return non_lvalue_loc (loc, res_op0);\n");
> +		fprintf (f, "  res = non_lvalue_loc (loc, res_op0);\n");
>  	      else
>  		{
>  		  if (e->operation->kind == id_base::CODE)
> -		    fprintf (f, "  return fold_build%d_loc (loc, %s, type",
> +		    fprintf (f, "  res = fold_build%d_loc (loc, %s, type",
>  			     e->ops.length (), e->operation->id);
>  		  else
> -		    fprintf (f, "  return build_call_expr_loc "
> +		    fprintf (f, "  res = build_call_expr_loc "
>  			     "(loc, builtin_decl_implicit (%s), %d",
>  			     e->operation->id, e->ops.length());
>  		  for (unsigned j = 0; j < e->ops.length (); ++j)
> @@ -2030,13 +2101,29 @@ dt_simplify::gen (FILE *f, bool gimple)
>  	}
>        else if (result->type == operand::OP_CAPTURE
>  	       || result->type == operand::OP_C_EXPR)
> +
>  	{
>  	  fprintf (f, "  tree res;\n");
>  	  s->result->gen_transform (f, " res", false, 1, "type", indexes);
> -	  fprintf (f, "  return res;\n");
>  	}
>        else
>  	gcc_unreachable ();
> +      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 (unsigned i = 0; i < n_braces; ++i)
> @@ -2107,18 +2194,6 @@ decision_tree::gen_generic (FILE *f)
>        fprintf (f, ")\n");
>        fprintf (f, "{\n");
>  
> -      /* ???  For now reject all simplifications on operands with
> -         side-effects as we are not prepared to properly wrap
> -	 omitted parts with omit_one_operand and friends.  In
> -	 principle we can do that automagically for a subset of
> -	 transforms (and only reject the remaining cases).
> -	 This fixes for example gcc.c-torture/execute/20050131-1.c.  */
> -      fprintf (f, "if ((op0 && TREE_SIDE_EFFECTS (op0))");
> -      for (unsigned i = 1; i < n; ++i)
> -	fprintf (f, "|| (op%d && TREE_SIDE_EFFECTS (op%d))", i, i);
> -      fprintf (f, ")\n"
> -	       "  return NULL_TREE;\n");
> -
>        fprintf (f, "switch (code)\n"
>  	       "{\n");
>        for (unsigned i = 0; i < root->kids.length (); i++)
>
Jakub Jelinek Oct. 22, 2014, 3:08 p.m. UTC | #2
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
Richard Biener Oct. 22, 2014, 6:57 p.m. UTC | #3
On October 22, 2014 5:08:50 PM CEST, Jakub Jelinek <jakub@redhat.com> wrote:
>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.

Yeah - this was by no means supposed to be an optimal solution.  But I just noticed we can fail to identify leafs as not all leafs need to be captured. Also I forgot to check for captures in with-exprs which can be used to transfer parts of the expression as well.

Meanwhile chasing another latent bug...

Richard.

>> @@ -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
diff mbox

Patch

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;
+      for (unsigned i = 0; i < e->ops.length (); ++i)
+	if (!count_captures (e->ops[i], cpt))
+	  return false;
+    }
+  else if (c_expr *e = dyn_cast <c_expr *> (o))
+    {
+      /* Give up for C exprs mentioning captures.  */
+      for (unsigned i = 0; i < e->code.length (); ++i)
+	if (e->code[i].type == CPP_ATSIGN
+	    && (e->code[i+1].type == CPP_NUMBER
+		|| e->code[i+1].type == CPP_NAME)
+	    && !(e->code[i+1].flags & PREV_WHITE))
+	  return false;
+    }
+  else
+    gcc_unreachable ();
+  return true;
+}
+
 /* Generate code for the '(if ...)', '(with ..)' and actual transform
    step of a '(simplify ...)' or '(match ...)'.  This handles everything
    that is not part of the decision tree (simplify->match).  */
@@ -1929,6 +1969,25 @@  dt_simplify::gen (FILE *f, bool gimple)
       n_braces++;
     }
 
+  /* For GENERIC we have to take care of wrapping multiple-used
+     expressions with side-effects in save_expr and preserve side-effects
+     of expressions with omit_one_operand.  Try to compute the number
+     of times an expression is substituted.  */
+  int *cpt = XALLOCAVEC (int, s->capture_max + 1);
+  memset (cpt, 0, sizeof (int) * (s->capture_max + 1));
+  if (!gimple
+      && s->result
+      && !count_captures (s->result, cpt))
+    {
+      /* Oops.  We could not get an exact count - give up and simply
+         reject the pattern if any input operand had side-effects.  */
+      fprintf (f, "if (TREE_SIDE_EFFECTS (op0)");
+      for (unsigned i = 1; i < as_a <expr *> (s->match)->ops.length (); ++i)
+	fprintf (f, " || TREE_SIDE_EFFECTS (op%u)", i);
+      fprintf (f, ") return NULL_TREE;\n");
+      cpt[0] = -1;
+    }
+
   fprintf (f, "if (dump_file && (dump_flags & TDF_DETAILS)) "
 	   "fprintf (dump_file, \"Applying pattern ");
   output_line_directive (f, s->result_location, true);
@@ -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 (unsigned j = 0; j < e->ops.length (); ++j)
 	    {
 	      char dest[32];
@@ -2004,22 +2074,23 @@  dt_simplify::gen (FILE *f, bool gimple)
 				    ? NULL : "TREE_TYPE (res_op0)");
 	      e->ops[j]->gen_transform (f, dest, false, 1, optype, indexes);
 	    }
-	  if (is_a <predicate_id *> (e->operation))
+	  if (is_predicate)
 	    fprintf (f, "return true;\n");
 	  else
 	    {
+	      fprintf (f, "  tree res;\n");
 	      /* Re-fold the toplevel result.  Use non_lvalue to
 	         build NON_LVALUE_EXPRs so they get properly
 		 ignored when in GIMPLE form.  */
 	      if (*e->operation == NON_LVALUE_EXPR)
-		fprintf (f, "  return non_lvalue_loc (loc, res_op0);\n");
+		fprintf (f, "  res = non_lvalue_loc (loc, res_op0);\n");
 	      else
 		{
 		  if (e->operation->kind == id_base::CODE)
-		    fprintf (f, "  return fold_build%d_loc (loc, %s, type",
+		    fprintf (f, "  res = fold_build%d_loc (loc, %s, type",
 			     e->ops.length (), e->operation->id);
 		  else
-		    fprintf (f, "  return build_call_expr_loc "
+		    fprintf (f, "  res = build_call_expr_loc "
 			     "(loc, builtin_decl_implicit (%s), %d",
 			     e->operation->id, e->ops.length());
 		  for (unsigned j = 0; j < e->ops.length (); ++j)
@@ -2030,13 +2101,29 @@  dt_simplify::gen (FILE *f, bool gimple)
 	}
       else if (result->type == operand::OP_CAPTURE
 	       || result->type == operand::OP_C_EXPR)
+
 	{
 	  fprintf (f, "  tree res;\n");
 	  s->result->gen_transform (f, " res", false, 1, "type", indexes);
-	  fprintf (f, "  return res;\n");
 	}
       else
 	gcc_unreachable ();
+      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 (unsigned i = 0; i < n_braces; ++i)
@@ -2107,18 +2194,6 @@  decision_tree::gen_generic (FILE *f)
       fprintf (f, ")\n");
       fprintf (f, "{\n");
 
-      /* ???  For now reject all simplifications on operands with
-         side-effects as we are not prepared to properly wrap
-	 omitted parts with omit_one_operand and friends.  In
-	 principle we can do that automagically for a subset of
-	 transforms (and only reject the remaining cases).
-	 This fixes for example gcc.c-torture/execute/20050131-1.c.  */
-      fprintf (f, "if ((op0 && TREE_SIDE_EFFECTS (op0))");
-      for (unsigned i = 1; i < n; ++i)
-	fprintf (f, "|| (op%d && TREE_SIDE_EFFECTS (op%d))", i, i);
-      fprintf (f, ")\n"
-	       "  return NULL_TREE;\n");
-
       fprintf (f, "switch (code)\n"
 	       "{\n");
       for (unsigned i = 0; i < root->kids.length (); i++)