diff mbox

[committed] Re: C PATCH to kill c_save_expr or towards delayed folding for the C FE

Message ID 20170522190933.GI8499@tucnak
State New
Headers show

Commit Message

Jakub Jelinek May 22, 2017, 7:09 p.m. UTC 
Hi!

On Fri, May 12, 2017 at 09:48:28PM +0200, Jakub Jelinek wrote:
> On Fri, May 12, 2017 at 09:37:27PM +0200, Marek Polacek wrote:
> > @@ -565,6 +564,25 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
> >  	 appropriate in any particular case.  */
> >        gcc_unreachable ();
> >  
> > +    case SAVE_EXPR:
> > +      /* Make sure to fold the contents of a SAVE_EXPR exactly once.  */
> > +      if (!SAVE_EXPR_FOLDED_P (expr))
> > +	{
> > +	  op0 = TREE_OPERAND (expr, 0);
> > +	  op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
> > +				       maybe_const_itself, for_int_const);
> > +	  /* Don't wrap the folded tree in a SAVE_EXPR if we don't
> > +	     have to.  */
> > +	  if (tree_invariant_p (op0))
> > +	    ret = op0;
> > +	  else
> > +	    {
> > +	      TREE_OPERAND (expr, 0) = op0;
> > +	      SAVE_EXPR_FOLDED_P (expr) = true;
> > +	    }
> > +	}
> 
> Wouldn't it be better to guard with if (!SAVE_EXPR_FOLDED_P (expr))
> only c_fully_fold_internal recursion on the operand
> and then use if (tree_invariant_p (op0)) unconditionally?

After discussions on IRC with Marek, we concluded the above is bad, because
if there is a huge SAVE_EXPR operand that c_fully_fold_internal folds into
an invariant (worse if it contains many other SAVE_EXPRs with such
properties), the current trunk code will fold it over and over again.

Fixed thusly, acked by Marek on IRC, bootstrapped/regtested on x86_64-linux
and i686-linux, committed to trunk.

2017-05-22  Jakub Jelinek  <jakub@redhat.com>

	* c-fold.c (c_fully_fold_internal): Save the c_fully_fold_internal
	result for SAVE_EXPR operand and set SAVE_EXPR_FOLDED_P even if
	it returned invariant.  Call tree_invariant_p unconditionally
	afterwards to decide whether to return expr or op0.



	Jakub
diff mbox

Patch

--- gcc/c/c-fold.c	2017-05-22 10:49:38.669161353 +0200
+++ gcc/c/c-fold.c	2017-05-22 18:09:49.358107221 +0200
@@ -566,21 +566,17 @@  c_fully_fold_internal (tree expr, bool i
 
     case SAVE_EXPR:
       /* Make sure to fold the contents of a SAVE_EXPR exactly once.  */
+      op0 = TREE_OPERAND (expr, 0);
       if (!SAVE_EXPR_FOLDED_P (expr))
 	{
-	  op0 = TREE_OPERAND (expr, 0);
 	  op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
 				       maybe_const_itself, for_int_const);
-	  /* Don't wrap the folded tree in a SAVE_EXPR if we don't
-	     have to.  */
-	  if (tree_invariant_p (op0))
-	    ret = op0;
-	  else
-	    {
-	      TREE_OPERAND (expr, 0) = op0;
-	      SAVE_EXPR_FOLDED_P (expr) = true;
-	    }
+	  TREE_OPERAND (expr, 0) = op0;
+	  SAVE_EXPR_FOLDED_P (expr) = true;
 	}
+      /* Return the SAVE_EXPR operand if it is invariant.  */
+      if (tree_invariant_p (op0))
+	ret = op0;
       goto out;
 
     default: