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 <ja...@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.
--- 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:
Jakub
