This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add save_expr langhook (PR c/68513)
- From: Richard Biener <rguenther at suse dot de>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Joseph Myers <joseph at codesourcery dot com>, Jakub Jelinek <jakub at redhat dot com>, ebotcazou at adacore dot com
- Date: Mon, 30 Nov 2015 17:00:09 +0100 (CET)
- Subject: Re: [PATCH] Add save_expr langhook (PR c/68513)
- Authentication-results: sourceware.org; auth=none
- References: <20151127185531 dot GA28072 at redhat dot com> <43651703-7ABE-491D-8D5B-863E921EA365 at suse dot de> <20151130154143 dot GF28072 at redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1511301648220 dot 4884 at t29 dot fhfr dot qr>
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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)