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?
I don't see why that would be better. It would mean calling tree_invariant_p
on even folded SAVE_EXPRs, and I can't see when it could be true in that case?
> > @@ -113,6 +113,10 @@ along with GCC; see the file COPYING3. If not see
> > subexpression meaning it is not a constant expression. */
> > #define CONSTRUCTOR_NON_CONST(EXPR) TREE_LANG_FLAG_1 (CONSTRUCTOR_CHECK
> > (EXPR))
> >
> > +/* For a SAVE_EXPR, nonzero if the contents of the SAVE_EXPR have already
> > + been folded. */
>
> s/contents/operand/;s/have/has/ ?
Ok, if you prefer ;).
> Otherwise I'm all for this, but would like to give you and Joseph as C FE
> maintainers the last word on this.
Thanks,
Marek
