From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3602 invoked by alias); 15 Jul 2014 10:00:41 -0000 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 Received: (qmail 3591 invoked by uid 89); 15 Jul 2014 10:00:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: atrey.karlin.mff.cuni.cz Received: from atrey.karlin.mff.cuni.cz (HELO atrey.karlin.mff.cuni.cz) (195.113.26.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Jul 2014 10:00:38 +0000 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 4018) id C10B681B80; Tue, 15 Jul 2014 12:00:35 +0200 (CEST) Date: Tue, 15 Jul 2014 10:24:00 -0000 From: Jan Hubicka To: Martin =?iso-8859-2?Q?Li=B9ka?= Cc: Jan Hubicka , GCC Patches Subject: Re: [RFC, PATCH 1/n] IPA C++ refactoring Message-ID: <20140715100035.GA15470@atrey.karlin.mff.cuni.cz> References: <53BE7C94.3010909@suse.cz> <20140711100702.GA8908@atrey.karlin.mff.cuni.cz> <53C4044E.5030106@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53C4044E.5030106@suse.cz> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg01044.txt.bz2 > I tried to mark it as protected, by there's usage that blocks that: > > In file included from ../../gcc/symtab.c:40:0: > ../../gcc/cgraph.h: In member function ?void symtab_node::unregister()?: > ../../gcc/cgraph.h:1178:16: error: ?cgraph_node* cgraph_node::find_replacement()? is protected > cgraph_node *find_replacement (void); > ^ > ../../gcc/symtab.c:462:46: error: within this context > replacement_node = cnode->find_replacement (); OK, lets keep it public for now then. > >Likewise > create_version_clone_with_body calls: new_version_node->call_function_insertion_hooks (); OK. > > Usages from outside of cgraph_node: > > cgraph.c:cgraph_node::create_empty (void) > cgraph.c: struct cgraph_node *node = cgraph_node::create_empty (); > cgraphclones.c: struct cgraph_node *new_node = cgraph_node::create_empty (); > lto-cgraph.c: node = cgraph_node::create_empty (); > > I will go through class members and check grouping one more. > What do you think about 'static' class functions, should be placed after all member functions? Sounds fine for me. Honza > > Thanks, > Martin > >+ > >+ /* Try to find a call graph node for declaration DECL and if it does not > >+ exist or if it corresponds to an inline clone, create a new one. */ > >+ static cgraph_node * get_create (tree); > > > >Also to begginig, next to get and create. > >+ > >+ /* Return the cgraph node that has ASMNAME for its DECL_ASSEMBLER_NAME. > >+ Return NULL if there's no such node. */ > >+ static cgraph_node *get_for_asmname (tree asmname); > > > >Likewise > >+ > >+ /* Attempt to mark ALIAS as an alias to DECL. Return alias node if > >+ successful and NULL otherwise. > >+ Same body aliases are output whenever the body of DECL is output, > >+ and cgraph_node::get (ALIAS) transparently > >+ returns cgraph_node::get (DECL). */ > >+ static cgraph_node * create_same_body_alias (tree alias, tree decl); > > > >To alias API > >+ > >+ /* Worker for cgraph_can_remove_if_no_direct_calls_p. */ > >+ static bool used_from_object_file_p_worker (cgraph_node *node, > >+ void *data ATTRIBUTE_UNUSED) > >+ { > >+ return node->used_from_object_file_p (); > >+ } > >+ > >+ /* Return true when cgraph_node can not be local. > >+ Worker for cgraph_local_node_p. */ > >cgraph_local_node_p was probably renamed, but we should sanify predicates here. > >Please group all functions dealing with local functions togehter, too. > >+ static bool non_local_p (cgraph_node *node, void *data ATTRIBUTE_UNUSED); > >+ > >+ /* Verify whole cgraph structure. */ > >+ static void DEBUG_FUNCTION verify_cgraph_nodes (void); > >+ > >+ /* Worker to bring NODE local. */ > >+ static bool make_local (cgraph_node *node, void *data ATTRIBUTE_UNUSED); > >+ > >+ /* Mark ALIAS as an alias to DECL. DECL_NODE is cgraph node representing > >+ the function body is associated > >+ with (not necessarily cgraph_node (DECL). */ > >+ static cgraph_node *create_alias (tree alias, tree target); > >+ > >+ static cgraph_edge * create_edge (cgraph_node *caller, cgraph_node *callee, > >+ gimple call_stmt, gcov_type count, > >+ int freq, > >+ bool indir_unknown_callee); > > > >Also edges and aliases should be grouped. > > > >With these changes patch looks good. Probably we will need one additional cleanup pass, but it would be better > >to do it incrementally. Please write changelog that carefuly records what was renamed to what. > > > >Honza