On Mon, 30 Nov 2015, Richard Biener wrote: > On Mon, 30 Nov 2015, Marek Polacek wrote: > > > On Sat, Nov 28, 2015 at 08:50:12AM +0100, Richard Biener wrote: > > > Different approach: after the FE folds (unexpectedly?), scan the result > > > for > > > SAVE_EXPRs and if found, drop the folding. > > > > Neither this fixes this problem completely, because we simply don't know > > where > > those SAVE_EXPRs might be introduced: it might be convert(), but e.g. when I > > changed the original testcase a tiny bit (added -), then those SAVE_EXPRs > > were > > introduced in a different spot (via c_process_stmt_expr -> c_fully_fold). > > So the following "disables" save_expr generation from generic-match.c > by failing to simplify if save_expr would end up not returning a > non-save_expr. > > I expect this will make fixing PR68590 difficult (w/o re-introducing > some fold-const.c code or changing genmatch to "special-case" > things). > > The other option for this PR is to re-introduce the TREE_SIDE_EFFECTS > check I removed earlier (to avoid un-CSEing large expressions at > -O0 for example) and thus only FAIL if the save_expr were needed > for correctness.
And the following will avoid quite some fallout (eventually). Testing as desired change independently. Richard. Index: gcc/match.pd =================================================================== --- gcc/match.pd (revision 231065) +++ gcc/match.pd (working copy) @@ -1828,15 +1828,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* Simplify comparison of something with itself. For IEEE floating-point, we can only do some of these simplifications. */ -(simplify - (eq @0 @0) - (if (! FLOAT_TYPE_P (TREE_TYPE (@0)) - || ! HONOR_NANS (TYPE_MODE (TREE_TYPE (@0)))) - { constant_boolean_node (true, type); })) -(for cmp (ge le) +(for cmp (eq ge le) (simplify (cmp @0 @0) - (eq @0 @0))) + (if (! FLOAT_TYPE_P (TREE_TYPE (@0)) + || ! HONOR_NANS (TYPE_MODE (TREE_TYPE (@0)))) + { constant_boolean_node (true, type); } + (if (cmp != EQ_EXPR) + (eq @0 @0))))) (for cmp (ne gt lt) (simplify (cmp @0 @0) > Richard. > > Index: gcc/tree.c > =================================================================== > --- gcc/tree.c (revision 231065) > +++ gcc/tree.c (working copy) > @@ -3231,8 +3231,6 @@ decl_address_ip_invariant_p (const_tree > not handle arithmetic; that's handled in skip_simple_arithmetic and > tree_invariant_p). */ > > -static bool tree_invariant_p (tree t); > - > static bool > tree_invariant_p_1 (tree t) > { > @@ -3282,7 +3280,7 @@ tree_invariant_p_1 (tree t) > > /* Return true if T is function-invariant. */ > > -static bool > +bool > tree_invariant_p (tree t) > { > tree inner = skip_simple_arithmetic (t); > Index: gcc/tree.h > =================================================================== > --- gcc/tree.h (revision 231065) > +++ gcc/tree.h (working copy) > @@ -4320,6 +4320,10 @@ extern tree staticp (tree); > > extern tree save_expr (tree); > > +/* Return true if T is function-invariant. */ > + > +extern bool tree_invariant_p (tree); > + > /* Look inside EXPR into any simple arithmetic operations. Return the > outermost non-arithmetic or non-invariant node. */ > > Index: gcc/genmatch.c > =================================================================== > --- gcc/genmatch.c (revision 231065) > +++ gcc/genmatch.c (working copy) > @@ -3106,7 +3106,9 @@ dt_simplify::gen_1 (FILE *f, int indent, > else if (is_a <predicate_id *> (opr)) > is_predicate = true; > /* Search for captures used multiple times in the result expression > - and dependent on TREE_SIDE_EFFECTS emit a SAVE_EXPR. */ > + and check if we can safely evaluate it multiple times. Otherwise > + fail, avoiding a SAVE_EXPR because that confuses the C FE > + const expression folding. */ > if (!is_predicate) > for (int i = 0; i < s->capture_max + 1; ++i) > { > @@ -3114,8 +3116,8 @@ dt_simplify::gen_1 (FILE *f, int indent, > continue; > if (cinfo.info[i].result_use_count > 1) > fprintf_indent (f, indent, > - "captures[%d] = save_expr (captures[%d]);\n", > - i, i); > + "if (! tree_invariant_p (captures[%d])) " > + "return NULL_TREE;\n", i); > } > for (unsigned j = 0; j < e->ops.length (); ++j) > { > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)