Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(124)

Issue 5701065: User directed Multiversioning support via Function Overloading

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 8 months ago by Sriraman
Modified:
13 years, 8 months ago
Reviewers:
davidxl
CC:
c-compiler-opt_google.com
Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/branches/google/main/gcc/
Visibility:
Public.

Description

Hi I have attached a patch to do user directed function multiversioning. This is still in prototyping and I want to send this upstream for feedback. I wanted to first get some feedback from the team. Thanks, -Sri. User directed Function Multiversioning (MV) via Function Overloading ==================================================================== This patch adds support for user directed function MV via function overloading. Here is an example program with function versions: int foo (); /* Default version */ int foo () __attribute__ ((targetv("arch=corei7")));/*Specialized for corei7 */ int foo () __attribute__ ((targetv("arch=core2")));/*Specialized for core2 */ int main () { int (*p)() = &foo; return foo () + (*p)(); } int foo () { return 0; } int __attribute__ ((targetv("arch=corei7"))) foo () { return 0; } int __attribute__ ((targetv("arch=core2"))) foo () { return 0; } The above example has foo defined 3 times, but all 3 definitions of foo are different versions of the same function. The call to foo in main, directly and via a pointer, are calls to the multi-versioned function foo which is dispatched to the right foo at run-time. Function versions must have the same signature but must differ in the specifier string provided to a new attribute called "targetv", which is nothing but the target attribute with an extra specification to indicate a version. Any number of versions can be created using the targetv attribute but it is mandatory to have one function without the attribute, which is treated as the default version. The dispatching is done using the IFUNC mechanism to keep the dispatch overhead low. The compiler creates a dispatcher function which checks the CPU type and calls the right version of foo. The dispatching code checks for the platform type and calls the first version that matches. The default function is called if no specialized version is appropriate for execution. The pointer to foo is made to be the address of the dispatcher function, so that it is unique and calls made via the pointer also work correctly. The assembler names of the various versions of foo is made different, by tagging the specifier strings, to keep them unique. A specific version can be called directly by creating an alias to its assembler name. For instance, to call the corei7 version directly, make an alias : int foo_corei7 () __attribute__((alias ("_Z3foov.arch_corei7"))); and then call foo_corei7. Note that using IFUNC blocks inlining of versioned functions. I had implemented an optimization earlier to do hot path cloning to allow versioned functions to be inlined. Please see : http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02285.html In the next iteration, I plan to merge these two. With that, hot code paths with versioned functions will be cloned so that versioned functions can be inlined.

Patch Set 1 #

Total comments: 11
Unified diffs Side-by-side diffs Delta from patch set Stats (+1249 lines, -7 lines) Patch
M Makefile.in View 2 chunks +6 lines, -0 lines 0 comments Download
M c-family/c-common.c View 3 chunks +47 lines, -0 lines 2 comments Download
M cgraphunit.c View 2 chunks +8 lines, -0 lines 0 comments Download
M config/i386/i386.c View 2 chunks +244 lines, -0 lines 0 comments Download
M config/i386/i386-builtin-types.def View 1 chunk +1 line, -0 lines 0 comments Download
M cp/call.c View 3 chunks +31 lines, -0 lines 0 comments Download
M cp/class.c View 5 chunks +37 lines, -3 lines 2 comments Download
M cp/decl.c View 6 chunks +38 lines, -2 lines 0 comments Download
M cp/decl2.c View 2 chunks +5 lines, -0 lines 0 comments Download
M doc/tm.texi View 1 chunk +8 lines, -0 lines 0 comments Download
M doc/tm.texi.in View 1 chunk +8 lines, -0 lines 0 comments Download
A multiversion.h View 1 chunk +51 lines, -0 lines 0 comments Download
A multiversion.c View 1 chunk +724 lines, -0 lines 7 comments Download
M passes.c View 1 chunk +1 line, -0 lines 0 comments Download
M target.def View 1 chunk +9 lines, -0 lines 0 comments Download
A testsuite/g++.dg/mv1.C View 1 chunk +23 lines, -0 lines 0 comments Download
M tree.h View 2 chunks +7 lines, -2 lines 0 comments Download
M tree-pass.h View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 4
davidxl
When submitting the patch upstream, you need to mention the current status with virtual member ...
13 years, 8 months ago (2012-02-28 07:26:25 UTC) #1
Sriraman
Hi David, Thanks for the quick review. Please find responses inlined. On 2012/02/28 07:26:25, davidxl ...
13 years, 8 months ago (2012-02-28 19:02:16 UTC) #2
davidxl
> > > Yes, '_' is better since demangler does not split it like '.' ...
13 years, 8 months ago (2012-02-28 19:29:40 UTC) #3
Sriraman
13 years, 8 months ago (2012-02-28 19:42:39 UTC) #4
On Tue, Feb 28, 2012 at 11:29 AM, Xinliang David Li <davidxl@google.com> wrote: >> >> >> Yes, '_' is better since demangler does not split it like '.' However, I >> should find a way to make this name unique. The current method does not >> guarantee that since source files path1/path2/foo.cc and >> path1_path2_foo.cc will get the same full path name under this scheme. > > path1.path2.foo.cc is also valid file name, right? if you include the > decl uids of all the version decls (including the default one), it is > very unlikely to get a conflict. Ok, I will change that. Paul told me there is a way already to generate a unique name. I am looking at that. > > For cases where the default func is public, should the resolver > function be made comdat? Ah, yes! Thanks. > >> >> >> >>> http://codereview.appspot.com/5701065/diff/1/multiversion.c#newcode578 >>> multiversion.c:578: ifunc_name = make_name (decl, "ifunc", >> >> append_filename); >>> >>> Shouldn't the ifunc_func inherit the name and assembler name from the >> >> original >>> >>> default version function (after the default function's name is >> >> changed) ? >> >> Actually, the default version function's name is not changed. Only the >> versions marked explicity with targetv get the name change. I can change >> the name of the default function and keep the ifunc name to be original >> name. > > This seems better as the default version function now becomes one of > the dispatch target. > >> >> It is a vector of version_function's not decls. So, I thought why define >> that struct in i386.c also. >> > > It might be better to use a VEC of trees instead of a linked list. Right, I will change this. Thanks, -Sri. > > > thanks, > > David > >> >> >>> http://codereview.appspot.com/5701065/diff/1/multiversion.c#newcode679 >>> multiversion.c:679: static unsigned int >>> Empty line after comment -- similarly for other places >> >> >> Thanks, >> -Sri. >> >> >> Description: >> Hi >> >> I have attached a patch to do user directed function multiversioning. >> This is still in prototyping and I want to send this upstream for >> feedback. I wanted  to first get some feedback from the team. >> >> Thanks, >> -Sri. >> >> User directed Function Multiversioning (MV) via Function Overloading >> ==================================================================== >> >> This patch adds support for user directed function MV via function >> overloading. >> >> Here is an example program with function versions: >> >> int foo ();  /* Default version */ >> int foo () __attribute__ ((targetv("arch=corei7")));/*Specialized for >> corei7 */ >> int foo () __attribute__ ((targetv("arch=core2")));/*Specialized for >> core2 */ >> >> int main () >> { >>  int (*p)() = &foo; >>  return foo () + (*p)(); >> } >> >> int foo () >> { >>  return 0; >> } >> >> int __attribute__ ((targetv("arch=corei7"))) >> foo () >> { >>  return 0; >> } >> >> int __attribute__ ((targetv("arch=core2"))) >> foo () >> { >>  return 0; >> } >> >> The above example has foo defined 3 times, but all 3 definitions of foo >> are different versions of the same function. The call to foo in main, >> directly and via a pointer, are calls to the multi-versioned function >> foo which is dispatched to the right foo at run-time. >> >> Function versions must have the same signature but must differ in the >> specifier string provided to a new attribute called "targetv", which is >> nothing but the target attribute with an extra specification to indicate >> a version. Any number of versions can be created using the targetv >> attribute but it is mandatory to have one function without the >> attribute, which is treated as the default version. >> >> The dispatching is done using the IFUNC mechanism to keep the dispatch >> overhead low. The compiler creates a dispatcher function which checks >> the CPU type and calls the right version of foo. The dispatching code >> checks for the platform >> type and calls the first version that matches. The default function is >> called if no specialized version is appropriate for execution. >> >> The pointer to foo is made to be the address of the dispatcher function, >> so that it is unique and calls made via the pointer also work correctly. >> The assembler names of the various versions of foo is made different, by >> tagging >> the specifier strings, to keep them unique.  A specific version can be >> called directly by creating an alias to its assembler name. For >> instance, to call the corei7 version directly, make an alias : >> int foo_corei7 () __attribute__((alias ("_Z3foov.arch_corei7"))); >> and then call foo_corei7. >> >> Note that using IFUNC  blocks inlining of versioned functions. I had >> implemented an optimization earlier to do hot path cloning to allow >> versioned functions to be >>  inlined. Please see : >> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02285.html >> In the next iteration, I plan to merge these two. With that, hot code >> paths with versioned functions will be cloned so that versioned >> functions can be inlined. >> >> Please review this at http://codereview.appspot.com/5701065/ >> >> Affected files: >>  M     Makefile.in >>  M     c-family/c-common.c >>  M     cgraphunit.c >>  M     config/i386/i386-builtin-types.def >>  M     config/i386/i386.c >>  M     cp/call.c >>  M     cp/class.c >>  M     cp/decl.c >>  M     cp/decl2.c >>  M     doc/tm.texi >>  M     doc/tm.texi.in >>  A     multiversion.c >>  A     multiversion.h >>  M     passes.c >>  M     target.def >>  A     testsuite/g++.dg/mv1.C >>  M     tree-pass.h >>  M     tree.h >> >> 
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b