From patchwork Thu Sep 23 10:15:42 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [PR,45565] Update SSA after stmt redirection From: Richard Guenther X-Patchwork-Id: 65522 Message-Id: To: GCC Patches , Jan Hubicka Date: Thu, 23 Sep 2010 12:15:42 +0200 On Thu, Sep 23, 2010 at 11:16 AM, Martin Jambor wrote: > Hi, > > the patch below fixes PR 45565 by updating SSA after inline transform > even if we don't inline anything and only redirect an edge.  I'm a bit > surprised this is necessary since we used to assert !need_ssa_update_p > in these cases > (http://gcc.gnu.org/ml/gcc-patches/2010-04/msg01012.html) ...but well, > in inline transform this does not matter. > > Bootstrapped and tested on x86_64-linux.  OK for trunk? No. The issue seems to be that we call update_stmt during gsi_replace with still the old fndecl on the stmt, and that old fndecl now became pure and thus that update_stmt dropped the VDEF. But the clone with the skipped args is _not_ marked pure for some reason and thus the following update_stmt gets back the VDEF but now requires renaming it. So I see a missed optimization here, and the PR can be fixed with instead. Testing that now. Richard. > Thanks, > > Martin > > > 2010-09-22  Martin Jambor   > >        PR middle-end/45565 >        * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Return NULL >        if nothing is changed. >        * tree-inline.c (copy_bb): Handle that >        cgraph_redirect_edge_call_stmt_to_callee can return NULL. >        * ipa-inline.c (inline_transform): Update SSA if >        cgraph_redirect_edge_call_stmt_to_callee returns non-NULL. > >        * testsuite/g++.dg/ipa/pr45565.C: New test. > > > Index: icln/gcc/cgraphunit.c > =================================================================== > --- icln.orig/gcc/cgraphunit.c > +++ icln/gcc/cgraphunit.c > @@ -2113,7 +2113,8 @@ cgraph_materialize_clone (struct cgraph_ >  } > >  /* If necessary, change the function declaration in the call statement > -   associated with E so that it corresponds to the edge callee.  */ > +   associated with E so that it corresponds to the edge callee.  Return the new > +   statement or NULL if nothing was changed.  */ > >  gimple >  cgraph_redirect_edge_call_stmt_to_callee (struct cgraph_edge *e) > @@ -2128,7 +2129,7 @@ cgraph_redirect_edge_call_stmt_to_callee >       || decl == e->callee->decl >       /* Don't update call from same body alias to the real function.  */ >       || (decl && cgraph_get_node (decl) == cgraph_get_node (e->callee->decl))) > -    return e->call_stmt; > +    return NULL; > >  #ifdef ENABLE_CHECKING >   if (decl) > Index: icln/gcc/ipa-inline.c > =================================================================== > --- icln.orig/gcc/ipa-inline.c > +++ icln/gcc/ipa-inline.c > @@ -2161,7 +2161,8 @@ inline_transform (struct cgraph_node *no > >   for (e = node->callees; e; e = e->next_callee) >     { > -      cgraph_redirect_edge_call_stmt_to_callee (e); > +      if (cgraph_redirect_edge_call_stmt_to_callee (e)) > +       todo = TODO_update_ssa; >       if (!e->inline_failed || warn_inline) >         inline_p = true; >     } > @@ -2169,7 +2170,7 @@ inline_transform (struct cgraph_node *no >   if (inline_p) >     { >       timevar_push (TV_INTEGRATION); > -      todo = optimize_inline_calls (current_function_decl); > +      todo |= optimize_inline_calls (current_function_decl); >       timevar_pop (TV_INTEGRATION); >     } >   cfun->always_inline_functions_inlined = true; > Index: icln/gcc/tree-inline.c > =================================================================== > --- icln.orig/gcc/tree-inline.c > +++ icln/gcc/tree-inline.c > @@ -1679,6 +1679,7 @@ copy_bb (copy_body_data *id, basic_block >                  if (edge) >                    { >                      int edge_freq = edge->frequency; > +                     gimple n_stmt; >                      edge = cgraph_clone_edge (edge, id->dst_node, stmt, >                                                gimple_uid (stmt), >                                                REG_BR_PROB_BASE, CGRAPH_FREQ_BASE, > @@ -1704,7 +1705,9 @@ copy_bb (copy_body_data *id, basic_block >                                   bb->frequency, >                                   copy_basic_block->frequency); >                        } > -                     stmt = cgraph_redirect_edge_call_stmt_to_callee (edge); > +                     n_stmt = cgraph_redirect_edge_call_stmt_to_callee (edge); > +                     if (n_stmt) > +                       stmt = n_stmt; >                    } >                  break; > > Index: icln/gcc/testsuite/g++.dg/ipa/pr45565.C > =================================================================== > --- /dev/null > +++ icln/gcc/testsuite/g++.dg/ipa/pr45565.C > @@ -0,0 +1,28 @@ > +// { dg-do compile } > +// { dg-options "-O -fno-toplevel-reorder -fno-inline -fipa-cp -fipa-cp-clone -fkeep-inline-functions" } > + > +template < typename Derived > struct AnyMatrixBase > +{ > +}; > + > +struct Matrix Random (); > + > +struct Matrix:AnyMatrixBase < Matrix > > +{ > +  void bar () > +  { > +    throw; > +  } > +  void foo (Matrix other) > +  { > +    bar (); > +    Matrix (AnyMatrixBase < Matrix > (Random ())); > +  } > +  template > +    < typename OtherDerived > Matrix (AnyMatrixBase < OtherDerived > other) > +  { > +    foo (other); > +  } > +}; > + > +Matrix x (Random ()); > Index: cgraphunit.c =================================================================== --- cgraphunit.c (revision 164550) +++ cgraphunit.c (working copy) @@ -2159,6 +2159,7 @@ cgraph_redirect_edge_call_stmt_to_callee new_stmt = gimple_call_copy_skip_args (e->call_stmt, e->callee->clone.combined_args_to_skip); + gimple_call_set_fndecl (new_stmt, e->callee->decl); if (gimple_vdef (new_stmt) && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME) @@ -2168,10 +2169,11 @@ cgraph_redirect_edge_call_stmt_to_callee gsi_replace (&gsi, new_stmt, true); } else - new_stmt = e->call_stmt; - - gimple_call_set_fndecl (new_stmt, e->callee->decl); - update_stmt (new_stmt); + { + new_stmt = e->call_stmt; + gimple_call_set_fndecl (new_stmt, e->callee->decl); + update_stmt (new_stmt); + } cgraph_set_call_stmt_including_clones (e->caller, e->call_stmt, new_stmt);