diff mbox

Fix pattern causing C_MAYBE_CONST_EXPRs leak into gimplifier (PR c/68513)

Message ID alpine.LSU.2.11.1511251558140.4884@t29.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Nov. 25, 2015, 3:02 p.m. UTC
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.

              }
          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.
diff mbox

Patch

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);