On Wed, 25 Nov 2015, Marek Polacek wrote:
> On Wed, Nov 25, 2015 at 03:45:08PM +0100, Richard Biener wrote:
> > But the whole point of the SAVE_EXPR is that it does _not_ "duplicate" it,
> > it just creates another use of the same value.
>
> Of course, but when 'x' in that pattern doesn't have side-effects, it's not
> wrapped in SAVE_EXPR and gets duplicated, generating unnecessary code, this
> is when I think the pattern is harmful.
Ok, I see what you mean. Yes, genmatch only inserts save_exprs
when required for correctness. But we can easily change that, making
the c_fully_fold issue possibly worse of course.
Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c (revision 230858)
+++ gcc/genmatch.c (working copy)
@@ -3112,16 +3111,10 @@ dt_simplify::gen_1 (FILE *f, int indent,
{
if (cinfo.info[i].same_as != (unsigned)i)
continue;
- if (!cinfo.info[i].force_no_side_effects_p
- && cinfo.info[i].result_use_count > 1)
- {
- fprintf_indent (f, indent,
- "if (TREE_SIDE_EFFECTS
(captures[%d]))\n",
- i);
- fprintf_indent (f, indent,
- " captures[%d] = save_expr
(captures[%d]);\n",
- i, i);
- }
+ if (cinfo.info[i].result_use_count > 1)
+ fprintf_indent (f, indent,
+ "captures[%d] = save_expr
(captures[%d]);\n",
+ i, i);
}
for (unsigned j = 0; j < e->ops.length (); ++j)
{
> > No. If c_fully_fold can't handle SAVE_EXPRs then maybe c_gimplify_expr
> > can simply strip them.
>
> Uhm, can we just strip SAVE_EXPRs like that? That sounds wrong. Did you
> mean C_MAYBE_CONST_EXPR?
Yes, strip C_MAYBE_CONST_EXPR but inside SAVE_EXPRs of course.
Richard.
