From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4707 invoked by alias); 26 Oct 2012 15:55:00 -0000 Received: (qmail 4678 invoked by uid 22791); 26 Oct 2012 15:54:58 -0000 X-SWARE-Spam-Status: No, hits=-0.8 required=5.0 tests=AWL,BAYES_00,KHOP_BIG_TO_CC,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from atrey.karlin.mff.cuni.cz (HELO atrey.karlin.mff.cuni.cz) (195.113.26.193) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 26 Oct 2012 15:54:49 +0000 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 4018) id 6CD60F069D; Fri, 26 Oct 2012 17:54:47 +0200 (CEST) Date: Fri, 26 Oct 2012 16:53:00 -0000 From: Jan Hubicka To: Sriraman Tallam Cc: Diego Novillo , Jason Merrill , Jan Hubicka , Xinliang David Li , mark@codesourcery.com, nathan@codesourcery.com, "H.J. Lu" , Richard Guenther , Uros Bizjak , reply@codereview.appspotmail.com, GCC Patches Subject: Re: User directed Function Multiversioning via Function Overloading (issue5752064) Message-ID: <20121026155447.GA4348@atrey.karlin.mff.cuni.cz> References: <5008708E.1030109@redhat.com> <506F27AF.3070805@redhat.com> <50816D63.3020908@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: 2012-10/txt/msg02434.txt.bz2 Hi, sorry for jumping in late, for too long I did not had chnce to look at my TODO. I have two comments... > Index: gcc/cgraphbuild.c > =================================================================== > --- gcc/cgraphbuild.c (revision 192623) > +++ gcc/cgraphbuild.c (working copy) > @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see > #include "ipa-utils.h" > #include "except.h" > #include "ipa-inline.h" > +#include "target.h" > > /* Context of record_reference. */ > struct record_reference_ctx > @@ -317,8 +318,23 @@ build_cgraph_edges (void) > bb); > decl = gimple_call_fndecl (stmt); > if (decl) > - cgraph_create_edge (node, cgraph_get_create_node (decl), > - stmt, bb->count, freq); > + { > + struct cgraph_node *callee = cgraph_get_create_node (decl); > + /* If a call to a multiversioned function dispatcher is > + found, generate the body to dispatch the right function > + at run-time. */ > + if (callee->dispatcher_function) > + { > + tree resolver_decl; > + gcc_assert (callee->function_version > + && callee->function_version->next); > + gcc_assert (targetm.generate_version_dispatcher_body); > + resolver_decl > + = targetm.generate_version_dispatcher_body (callee); > + gcc_assert (resolver_decl != NULL_TREE); > + } > + cgraph_create_edge (node, callee, stmt, bb->count, freq); > + } I do not really think resolver generation belongs here + I would preffer build_cgraph_edges to really just build the edges. > Index: gcc/cgraph.c > =================================================================== > --- gcc/cgraph.c (revision 192623) > +++ gcc/cgraph.c (working copy) > @@ -1277,6 +1277,16 @@ cgraph_mark_address_taken_node (struct cgraph_node > node->symbol.address_taken = 1; > node = cgraph_function_or_thunk_node (node, NULL); > node->symbol.address_taken = 1; > + /* If the address of a multiversioned function dispatcher is taken, > + generate the body to dispatch the right function at run-time. This > + is needed as the address can be used to do an indirect call. */ > + if (node->dispatcher_function) > + { > + gcc_assert (node->function_version > + && node->function_version->next); > + gcc_assert (targetm.generate_version_dispatcher_body); > + targetm.generate_version_dispatcher_body (node); > + } Similarly here. I also think this way you will miss aliases of the multiversioned functions. I am not sure why the multiversioning is tied with the cgraph build and the datastructure is put into cgraph_node itself. It seems to me that your dispatchers are in a way related to thunks - i.e. they are inserted into callgraph and once they become reachable their body needs to be produced. I think generate_version_dispatcher_body should thus probably be done from cgraph_analyze_function. (to make the function to be seen by analyze_function you will need to make it to be finalized at the time you set dispatcher_function flag. I would also put the dispatcher datastructure into on-side hash by node->uid. (i.e. these are rare and thus the datastructure should be small) symbol table is critical for WPA stage memory use and I plan to remove as much as possible from the nodes in near future. For this reason I would preffer to not add too much of stuff that is not going to be used by majority of nodes. Honza