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)

Reply via email to