[PATCH] Fix fold_stmt ICE (PR c++/49264)

Richard Guenther richard.guenther@gmail.com
Mon Jun 6 09:30:00 GMT 2011


On Fri, Jun 3, 2011 at 3:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> If memcpy is folded into an assignment, that assignment can be for C++
> folded into nothing (if it is copying of 1 byte from or to empty C++ class).
> gimple-fold.c was changed to handle that case in some spots, but not all,
> particularly if that memcpy is the last stmt in a basic block (which can
> happen e.g. during inlining), gsi_stmt (*gsi) in fold_stmt_1 will ICE.
> The gimple-fold.c hunks fix this (and also make sure that even when it
> isn't the last stmt in a bb, we won't try to fold lhs of next stmt
> if it folded/gimplified into nothing).  With that fix the testcase ICEs
> shortly afterwards, the tree-inline.c hunk is supposed to fix that.
> I'd use cgraph_remove_edge there, but I'd have to handle all clones too,
> so I'm calling cgraph_update_edges_for_call_stmt with a dummy stmt
> instead (perhaps NULL could be used instead with adjustment of the
> cgraph_update_edges_for_call_stmt{,_node} routines?).
> The cgraph.c change is just in case the edge isn't there, we'd do useless
> work and crash while doing that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?

Ok, but ... Honza, can you suggest sth better for ...

> 2011-06-03  Jakub Jelinek  <jakub@redhat.com>
>
>        PR c++/49264
>        * gimple-fold.c (fold_stmt_1): Don't try to fold *& on the lhs
>        if stmt folded into nothing.
>        * tree-inline.c (fold_marked_statements): If a builtin at the
>        end of a bb folded into nothing, just update cgraph edges
>        and move to next bb.
>        * cgraph.c (cgraph_update_edges_for_call_stmt_node): Don't compute
>        count and frequency if new_call is NULL.
>
>        * g++.dg/opt/pr49264.C: New test.
>
> --- gcc/gimple-fold.c.jj        2011-05-24 23:34:28.000000000 +0200
> +++ gcc/gimple-fold.c   2011-06-03 09:13:34.000000000 +0200
> @@ -1577,6 +1577,11 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>   bool changed = false;
>   gimple stmt = gsi_stmt (*gsi);
>   unsigned i;
> +  gimple_stmt_iterator gsinext = *gsi;
> +  gimple next_stmt;
> +
> +  gsi_next (&gsinext);
> +  next_stmt = gsi_end_p (gsinext) ? NULL : gsi_stmt (gsinext);
>
>   /* Fold the main computation performed by the statement.  */
>   switch (gimple_code (stmt))
> @@ -1665,10 +1670,19 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
>     default:;
>     }
>
> +  /* If stmt folds into nothing and it was the last stmt in a bb,
> +     don't call gsi_stmt.  */
> +  if (gsi_end_p (*gsi))
> +    {
> +      gcc_assert (next_stmt == NULL);
> +      return changed;
> +    }
> +
>   stmt = gsi_stmt (*gsi);
>
> -  /* Fold *& on the lhs.  */
> -  if (gimple_has_lhs (stmt))
> +  /* Fold *& on the lhs.  Don't do this if stmt folded into nothing,
> +     as we'd changing the next stmt.  */
> +  if (gimple_has_lhs (stmt) && stmt != next_stmt)
>     {
>       tree lhs = gimple_get_lhs (stmt);
>       if (lhs && REFERENCE_CLASS_P (lhs))
> --- gcc/tree-inline.c.jj        2011-06-02 10:15:20.000000000 +0200
> +++ gcc/tree-inline.c   2011-06-03 09:29:15.000000000 +0200
> @@ -4108,6 +4108,14 @@ fold_marked_statements (int first, struc
>                  if (fold_stmt (&gsi))
>                    {
>                      gimple new_stmt;
> +                     /* If a builtin at the end of a bb folded into nothing,
> +                        the following loop won't work.  */
> +                     if (gsi_end_p (gsi))
> +                       {
> +                         cgraph_update_edges_for_call_stmt (old_stmt, old_decl,
> +                                                            gimple_build_nop ());

This?  Esp. I don't like the gimple_build_nop () here too much.

Thanks,
Richard.

> +                         break;
> +                       }
>                      if (gsi_end_p (i2))
>                        i2 = gsi_start_bb (BASIC_BLOCK (first));
>                      else
> --- gcc/cgraph.c.jj     2011-05-11 19:39:03.000000000 +0200
> +++ gcc/cgraph.c        2011-06-03 09:40:02.000000000 +0200
> @@ -1277,7 +1277,7 @@ cgraph_update_edges_for_call_stmt_node (
>          frequency = e->frequency;
>          cgraph_remove_edge (e);
>        }
> -      else
> +      else if (new_call)
>        {
>          /* We are seeing new direct call; compute profile info based on BB.  */
>          basic_block bb = gimple_bb (new_stmt);
> --- gcc/testsuite/g++.dg/opt/pr49264.C.jj       2011-06-03 09:35:50.000000000 +0200
> +++ gcc/testsuite/g++.dg/opt/pr49264.C  2011-06-03 08:50:33.000000000 +0200
> @@ -0,0 +1,19 @@
> +// PR c++/49264
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +struct B { };
> +struct A { char a[sizeof (B) + 1]; } a;
> +
> +static inline void
> +foo (const B &b)
> +{
> +  __builtin_memcpy (&a, &b, sizeof (b));
> +}
> +
> +void
> +bar ()
> +{
> +  B c;
> +  foo (c);
> +}
>
>        Jakub
>



More information about the Gcc-patches mailing list