From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27153 invoked by alias); 16 Dec 2010 17:32:02 -0000 Received: (qmail 27131 invoked by uid 22791); 16 Dec 2010 17:32:00 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI X-Spam-Check-By: sourceware.org Received: from cantor.suse.de (HELO mx1.suse.de) (195.135.220.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Dec 2010 17:31:54 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.suse.de (Postfix) with ESMTP id DEDC990847; Thu, 16 Dec 2010 18:31:51 +0100 (CET) Date: Thu, 16 Dec 2010 18:47:00 -0000 From: Martin Jambor To: Jan Hubicka Cc: gcc-patches@gcc.gnu.org Subject: Re: PR testsuite/45621 (indirect inlining related cgraph verifier ICE) Message-ID: <20101216173151.GB14911@virgil.arch.suse.de> Mail-Followup-To: Jan Hubicka , gcc-patches@gcc.gnu.org References: <20101015012325.GC15487@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20101015012325.GC15487@kam.mff.cuni.cz> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-12/txt/msg01319.txt.bz2 Hi, On Fri, Oct 15, 2010 at 03:23:25AM +0200, Jan Hubicka wrote: > Hi, > the testcase causes inliner confussion when folding statement that is call to a clone > of virtual method. It thinks that folding folded call of builtin and re-creates an edge. > This is wrong since inline plan is lost and we then run into verifier ICE. > > The patch makes inliner more cureful about deciding when is an original call and what > is new call introduced by builtin folding. > > Bootstrapped/regtested x86_64-linux, comitted. > Index: ChangeLog > =================================================================== > *** ChangeLog (revision 165491) > --- ChangeLog (working copy) > *************** > *** 1,3 **** > --- 1,12 ---- > + 2010-10-14 Jan Hubicka > + > + PR middle-end/45621 > + * cgraph.c (cgraph_update_edges_for_call_stmt_node): When new call is > + redirected to clone, be happy. > + * cgraph.h (cgraph node): Enable former_clone_of unconditinally. > + * cgraphunit.c (verify_cgraph_node, cgraph_materialize_clone): Handle > + former_clone_of unconditinally. > + I have just noticed that we still set former_clone_of in cgraph_remove_unreachable_nodes only when checking is enabled: #ifdef ENABLE_CHECKING if (node->clone_of) node->former_clone_of = node->clone_of->decl; #endif That does not sound right if there are more uses of former_clone_of chan checking now. Should we remove that #ifdef? Thanks, Martin > 2010-10-14 Iain Sandoe > > merge from FSF apple 'trunk' branch. > Index: testsuite/ChangeLog > =================================================================== > *** testsuite/ChangeLog (revision 165491) > --- testsuite/ChangeLog (working copy) > *************** > *** 1,3 **** > --- 1,10 ---- > + 2010-10-14 Jan Hubicka > + > + PR middle-end/45621 > + * g++.dg/lto/pr45621.h : New. > + * g++.dg/lto/pr45621_0.C: New. > + * g++.dg/lto/pr45621_1.C: New. > + > 2010-10-14 Iain Sandoe > > * objc.dg/property: New. > Index: cgraph.c > =================================================================== > *** cgraph.c (revision 165491) > --- cgraph.c (working copy) > *************** cgraph_update_edges_for_call_stmt_node ( > *** 1241,1249 **** > { > /* See if the edge is already there and has the correct callee. It > might be so because of indirect inlining has already updated > ! it. */ > ! if (new_call && e->callee && e->callee->decl == new_call) > ! return; > > /* Otherwise remove edge and create new one; we can't simply redirect > since function has changed, so inline plan and other information > --- 1241,1258 ---- > { > /* See if the edge is already there and has the correct callee. It > might be so because of indirect inlining has already updated > ! it. We also might've cloned and redirected the edge. */ > ! if (new_call && e->callee) > ! { > ! struct cgraph_node *callee = e->callee; > ! while (callee) > ! { > ! if (callee->decl == new_call > ! || callee->former_clone_of == new_call) > ! return; > ! callee = callee->clone_of; > ! } > ! } > > /* Otherwise remove edge and create new one; we can't simply redirect > since function has changed, so inline plan and other information > Index: cgraph.h > =================================================================== > *** cgraph.h (revision 165491) > --- cgraph.h (working copy) > *************** struct GTY((chain_next ("%h.next"), chai > *** 227,237 **** > /* For functions with many calls sites it holds map from call expression > to the edge to speed up cgraph_edge function. */ > htab_t GTY((param_is (struct cgraph_edge))) call_site_hash; > ! #ifdef ENABLE_CHECKING > ! /* Declaration node used to be clone of. Used for checking only. > ! We must skip it or we get references from release checking GGC files. */ > ! tree GTY ((skip)) former_clone_of; > ! #endif > > PTR GTY ((skip)) aux; > > --- 227,234 ---- > /* For functions with many calls sites it holds map from call expression > to the edge to speed up cgraph_edge function. */ > htab_t GTY((param_is (struct cgraph_edge))) call_site_hash; > ! /* Declaration node used to be clone of. */ > ! tree former_clone_of; > > PTR GTY ((skip)) aux; > > Index: cgraphunit.c > =================================================================== > *** cgraphunit.c (revision 165491) > --- cgraphunit.c (working copy) > *************** verify_cgraph_node (struct cgraph_node * > *** 656,662 **** > debug_tree (e->callee->decl); > error_found = true; > } > - #ifdef ENABLE_CHECKING > else if (!e->callee->global.inlined_to > && decl > && cgraph_get_node (decl) > --- 656,661 ---- > *************** verify_cgraph_node (struct cgraph_node * > *** 671,677 **** > debug_tree (decl); > error_found = true; > } > - #endif > } > else if (decl) > { > --- 670,675 ---- > *************** static void > *** 2079,2089 **** > cgraph_materialize_clone (struct cgraph_node *node) > { > bitmap_obstack_initialize (NULL); > - #ifdef ENABLE_CHECKING > node->former_clone_of = node->clone_of->decl; > if (node->clone_of->former_clone_of) > node->former_clone_of = node->clone_of->former_clone_of; > - #endif > /* Copy the OLD_VERSION_NODE function tree to the new version. */ > tree_function_versioning (node->clone_of->decl, node->decl, > node->clone.tree_map, true, > --- 2077,2085 ---- > Index: testsuite/g++.dg/lto/pr45621.h > =================================================================== > *** testsuite/g++.dg/lto/pr45621.h (revision 0) > --- testsuite/g++.dg/lto/pr45621.h (revision 0) > *************** > *** 0 **** > --- 1,8 ---- > + struct S > + { > + void m (); > + virtual void v1 (); > + virtual void v2 (); > + }; > + > + extern S s; > Index: testsuite/g++.dg/lto/pr45621_0.C > =================================================================== > *** testsuite/g++.dg/lto/pr45621_0.C (revision 0) > --- testsuite/g++.dg/lto/pr45621_0.C (revision 0) > *************** > *** 0 **** > --- 1,10 ---- > + // { dg-lto-do assemble } > + // { dg-extra-ld-options "-O2 -fipa-cp-clone -flto -nostdlib -r" } > + #include "pr45621.h" > + > + void > + foo () > + { > + s.v1 (); > + s.m (); > + } > Index: testsuite/g++.dg/lto/pr45621_1.C > =================================================================== > *** testsuite/g++.dg/lto/pr45621_1.C (revision 0) > --- testsuite/g++.dg/lto/pr45621_1.C (revision 0) > *************** > *** 0 **** > --- 1,13 ---- > + #include "pr45621.h" > + > + void > + S::v1 () > + { > + v2 (); > + } > + > + void > + S::m () > + { > + v1 (); > + }