This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]