From patchwork Thu Sep 23 10:15:42 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 65522 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 3A97EB70D6 for ; Thu, 23 Sep 2010 20:16:01 +1000 (EST) Received: (qmail 21880 invoked by alias); 23 Sep 2010 10:15:58 -0000 Received: (qmail 21863 invoked by uid 22791); 23 Sep 2010 10:15:56 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, TW_FN, TW_TM X-Spam-Check-By: sourceware.org Received: from mail-iw0-f175.google.com (HELO mail-iw0-f175.google.com) (209.85.214.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 23 Sep 2010 10:15:47 +0000 Received: by iwn2 with SMTP id 2so1450713iwn.20 for ; Thu, 23 Sep 2010 03:15:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.231.146.212 with SMTP id i20mr1866499ibv.52.1285236942218; Thu, 23 Sep 2010 03:15:42 -0700 (PDT) Received: by 10.231.17.130 with HTTP; Thu, 23 Sep 2010 03:15:42 -0700 (PDT) In-Reply-To: <20100923091657.GA31666@virgil.arch.suse.de> References: <20100923091657.GA31666@virgil.arch.suse.de> Date: Thu, 23 Sep 2010 12:15:42 +0200 Message-ID: Subject: Re: [PATCH, PR 45565] Update SSA after stmt redirection From: Richard Guenther To: GCC Patches , Jan Hubicka X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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);