| Message ID | CAO2gOZWQK1c_6w6R9Oa1XXvBR5R-==1ZpMHOA8A9dGVACp=oMg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Jun 3, 2012 at 11:17 PM, Dehao Chen <dehao@google.com> wrote: > I've updated the patch using deferred option. > > http://codereview.appspot.com/6281047 > > Thanks, > Dehao > > The new patch: > > 2012-06-01 Dehao Chen <dehao@google.com> > > * gcc/cgraph.c (cgraph_match_attribute_by_name): New function. > (cgraph_node): Add attribute to function decl. > * gcc/opts-global.c (add_attribute_list_to_vector): New function. > (handle_common_deferred_options): Handle new options. > * gcc/opts.c (common_handle_option): Handle new options. > * gcc/opts.h (attribute_pair): New type. > * gcc/common.opt (flag_function_attribute_list): New option. > Index: gcc/doc/invoke.texi > =================================================================== > --- gcc/doc/invoke.texi (revision 188050) > +++ gcc/doc/invoke.texi (working copy) > @@ -362,7 +362,8 @@ > -fdelete-null-pointer-checks -fdse -fdevirtualize -fdse @gol > -fearly-inlining -fipa-sra -fexpensive-optimizations -ffast-math @gol > -ffinite-math-only -ffloat-store -fexcess-precision=@var{style} @gol > --fforward-propagate -ffp-contract=@var{style} -ffunction-sections @gol > +-fforward-propagate -ffp-contract=@var{style} @gol > +-ffunction-attribute-list -ffunction-sections @gol > -fgcse -fgcse-after-reload -fgcse-las -fgcse-lm -fgraphite-identity @gol > -fgcse-sm -fif-conversion -fif-conversion2 -findirect-inlining @gol > -finline-functions -finline-functions-called-once -finline-limit=@var{n} @gol > @@ -8585,6 +8586,10 @@ > specify this option and you may have problems with debugging if > you specify both this option and @option{-g}. > > +@item -ffunction-attribute-list > +@opindex ffunction-attribute-list > +List of function name patterns that will be applied specified attribute. > + > @item -fbranch-target-load-optimize > @opindex fbranch-target-load-optimize > Perform branch target register load optimization before prologue / epilogue > Index: gcc/cgraph.c > =================================================================== > --- gcc/cgraph.c (revision 188050) > +++ gcc/cgraph.c (working copy) > @@ -99,6 +99,7 @@ > #include "ipa-utils.h" > #include "lto-streamer.h" > #include "l-ipo.h" > +#include "opts.h" > > const char * const ld_plugin_symbol_resolution_names[]= > { > @@ -520,6 +521,32 @@ > } > } > > +/* Match FNDECL's name with user specified patterns. If match is found, add > + attributes to FNDECL. > + name matches with pattern, iff one of the following conditions satisfy: > + 1. strcmp (name, pattern) != 0 > + 2. pattern[len - 1] = '*' && strncmp (name, pattern, len - 1) != 0 */ Fix the comment -- !strcmp ... > +static void > +cgraph_match_attribute_by_name (tree fndecl) > +{ > + unsigned i; > + attribute_pair_p p; > + const char *name = lang_hooks.decl_printable_name(fndecl, 0); It should probably use mangled name here. > + > + if (!name) > + return; > + > + FOR_EACH_VEC_ELT (attribute_pair_p, function_attribute_list, i, p) > + { > + char *n = p->name; > + int len = strlen (n); > + if ((n[len - 1] == '*' && !strncmp (name, n, len - 1)) > + || !strcmp (name, n)) > + decl_attributes (&fndecl, tree_cons ( > + get_identifier (p->attribute), NULL, NULL), 0); > + } > +} > + > /* Return cgraph node assigned to DECL. Create new one when needed. */ > > struct cgraph_node * > @@ -554,6 +581,7 @@ > node->origin->nested = node; > } > cgraph_add_assembler_hash_node (node); > + cgraph_match_attribute_by_name (decl); > return node; > } > > Index: gcc/opts.c > =================================================================== > --- gcc/opts.c (revision 188050) > +++ gcc/opts.c (working copy) > @@ -1647,6 +1647,10 @@ > /* Deferred. */ > break; > > + case OPT_ffunction_attribute_list_: > + /* Deferred. */ > + break; > + > case OPT_fsched_verbose_: > #ifdef INSN_SCHEDULING > /* Handled with Var in common.opt. */ > Index: gcc/opts.h > =================================================================== > --- gcc/opts.h (revision 188050) > +++ gcc/opts.h (working copy) > @@ -272,6 +272,15 @@ > struct cl_option_handler_func handlers[3]; > }; > > +typedef struct { > + char *name; > + char *attribute; > +} attribute_pair; > +typedef attribute_pair *attribute_pair_p; > +DEF_VEC_P(attribute_pair_p); > +DEF_VEC_ALLOC_P(attribute_pair_p,heap); > +extern VEC(attribute_pair_p,heap) *function_attribute_list; > + > /* Input file names. */ > > extern const char **in_fnames; > Index: gcc/common.opt > =================================================================== > --- gcc/common.opt (revision 188050) > +++ gcc/common.opt (working copy) > @@ -1242,6 +1242,10 @@ > Common Report Var(flag_function_sections) > Place each function into its own section > > +ffunction-attribute-list= > +Common Joined RejectNegative Var(common_deferred_options) Defer > +-ffunction-attribute-list=attribute:name,... Add attribute to named functions > + > fgcda= > Common Joined RejectNegative Var(gcov_da_name) > Set the gcov data file name. > Index: gcc/opts-global.c > =================================================================== > --- gcc/opts-global.c (revision 188050) > +++ gcc/opts-global.c (working copy) > @@ -50,6 +50,8 @@ > const char **in_fnames; > unsigned num_in_fnames; > > +VEC(attribute_pair_p,heap) *function_attribute_list; > + > /* Return a malloced slash-separated list of languages in MASK. */ > > static char * > @@ -79,6 +81,66 @@ > return result; > } > > +/* Add strings like attribute_str:name1,name2... to a char_pair_p vector. */ > + > +static void > +add_attribute_list_to_vector (void **pvec, const char *arg) > +{ > + char *tmp; > + char *r; > + char *w; > + char *token_start; > + char *attribute; > + VEC(attribute_pair_p,heap) *vec = (VEC(attribute_pair_p,heap) *) *pvec; > + > + /* We never free this string. */ > + tmp = xstrdup (arg); > + attribute = tmp; > + > + for (r = tmp; *r != '\0' && *r != ':'; ++r) > + ; > + > + if (*r != ':') > + return; Use 'strchr'. > + > + *r = '\0'; > + ++r; > + w = r; > + token_start = r; > + > + while (*r != '\0') > + { > + if (*r == ',') > + { > + attribute_pair_p p = > + (attribute_pair_p) xmalloc (sizeof (attribute_pair)); > + p->name = token_start; > + p->attribute = attribute; > + *w++ = '\0'; > + ++r; > + VEC_safe_push (attribute_pair_p, heap, vec, p); > + token_start = w; > + } > + if (*r == '\\' && r[1] == ',') > + { > + *w++ = ','; > + r += 2; > + } > + else > + *w++ = *r++; > + } Simplify the code with 'strchr'. thanks, David > + if (*token_start != '\0') > + { > + attribute_pair_p p = > + (attribute_pair_p) xmalloc (sizeof (attribute_pair)); > + p->name = token_start; > + p->attribute = attribute; > + VEC_safe_push (attribute_pair_p, heap, vec, p); > + } > + > + *pvec = vec; > +} > + > /* Complain that switch DECODED does not apply to this front end (mask > LANG_MASK). */ > > @@ -452,6 +514,10 @@ > set_random_seed (opt->arg); > break; > > + case OPT_ffunction_attribute_list_: > + add_attribute_list_to_vector(&function_attribute_list, opt->arg); > + break; > + > case OPT_fstack_limit: > /* The real switch is -fno-stack-limit. */ > if (!opt->value) > > On Sun, Jun 3, 2012 at 9:14 PM, Dehao Chen <dehao@google.com> wrote: >> Thank you guys for the comments, I'll update the patch to : >> >> 1. generalize the flag to enable other annotations such always_inline. >> 2. change to use deferred option. >> >> Thanks, >> Dehao >> >> On Sun, Jun 3, 2012 at 12:40 PM, Xinliang David Li <davidxl@google.com> wrote: >>> On Sat, Jun 2, 2012 at 11:11 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >>>>> Actually Dehao also plans to teach the static predictor to understand >>>>> standard library functions more (e.g IO functions) and add more naming >>>> >>>> How this differ from annotating the library? >>> >>> I find them more suitable to be compiler heuristic than being >>> function's attribute -- attribute is a much stronger assertion. >>> >>>> >>>> There are indeed quite some possibilities to do about library calls.... >>>> >>>> One thing I always wondered about is how to tell predictor that paths containing >>>> heavy IO functions don't need to be really opimized to death, since their execution >>>> time is dominated by the IO... >>>> >>> >>> Yes -- if branch predictor does the right thing and if function >>> splitter is powerful enough, the IO code can be outlined and optimized >>> for size :) >>> >>> >>>>> based heuristics such as 'error, success, failure, fatal etc). >>>> >>>> yeah, this is also mentioned by some branch prediction papers. It is bit kludgy >>>> in nature (i.e. it won't understand programs written in Czech language) but it >>>> is an heuristics after all. >>>> >>> >>> right. >>> >>> thanks, >>> >>> David >>> >>>> Honza >>>>> >>>>> thanks, >>>>> >>>>> David >>>>> >>>>> > Honza >>>>> >> >>>>> >> thanks, >>>>> >> >>>>> >> David >>>>> >> > Honza
Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 188050) +++ gcc/doc/invoke.texi (working copy) @@ -362,7 +362,8 @@ -fdelete-null-pointer-checks -fdse -fdevirtualize -fdse @gol -fearly-inlining -fipa-sra -fexpensive-optimizations -ffast-math @gol -ffinite-math-only -ffloat-store -fexcess-precision=@var{style} @gol --fforward-propagate -ffp-contract=@var{style} -ffunction-sections @gol +-fforward-propagate -ffp-contract=@var{style} @gol +-ffunction-attribute-list -ffunction-sections @gol -fgcse -fgcse-after-reload -fgcse-las -fgcse-lm -fgraphite-identity @gol -fgcse-sm -fif-conversion -fif-conversion2 -findirect-inlining @gol -finline-functions -finline-functions-called-once -finline-limit=@var{n} @gol @@ -8585,6 +8586,10 @@ specify this option and you may have problems with debugging if you specify both this option and @option{-g}. +@item -ffunction-attribute-list +@opindex ffunction-attribute-list +List of function name patterns that will be applied specified attribute. + @item -fbranch-target-load-optimize @opindex fbranch-target-load-optimize Perform branch target register load optimization before prologue / epilogue Index: gcc/cgraph.c =================================================================== --- gcc/cgraph.c (revision 188050) +++ gcc/cgraph.c (working copy) @@ -99,6 +99,7 @@ #include "ipa-utils.h" #include "lto-streamer.h" #include "l-ipo.h" +#include "opts.h" const char * const ld_plugin_symbol_resolution_names[]= { @@ -520,6 +521,32 @@ } } +/* Match FNDECL's name with user specified patterns. If match is found, add + attributes to FNDECL. + name matches with pattern, iff one of the following conditions satisfy: + 1. strcmp (name, pattern) != 0 + 2. pattern[len - 1] = '*' && strncmp (name, pattern, len - 1) != 0 */ +static void +cgraph_match_attribute_by_name (tree fndecl) +{ + unsigned i; + attribute_pair_p p; + const char *name = lang_hooks.decl_printable_name(fndecl, 0); + + if (!name) + return; + + FOR_EACH_VEC_ELT (attribute_pair_p, function_attribute_list, i, p) + { + char *n = p->name; + int len = strlen (n); + if ((n[len - 1] == '*' && !strncmp (name, n, len - 1)) + || !strcmp (name, n)) + decl_attributes (&fndecl, tree_cons ( + get_identifier (p->attribute), NULL, NULL), 0); + } +} + /* Return cgraph node assigned to DECL. Create new one when needed. */ struct cgraph_node * @@ -554,6 +581,7 @@ node->origin->nested = node; } cgraph_add_assembler_hash_node (node); + cgraph_match_attribute_by_name (decl); return node; } Index: gcc/opts.c =================================================================== --- gcc/opts.c (revision 188050) +++ gcc/opts.c (working copy) @@ -1647,6 +1647,10 @@ /* Deferred. */ break; + case OPT_ffunction_attribute_list_: + /* Deferred. */ + break; + case OPT_fsched_verbose_: #ifdef INSN_SCHEDULING /* Handled with Var in common.opt. */ Index: gcc/opts.h =================================================================== --- gcc/opts.h (revision 188050) +++ gcc/opts.h (working copy) @@ -272,6 +272,15 @@ struct cl_option_handler_func handlers[3]; }; +typedef struct { + char *name; + char *attribute; +} attribute_pair; +typedef attribute_pair *attribute_pair_p; +DEF_VEC_P(attribute_pair_p); +DEF_VEC_ALLOC_P(attribute_pair_p,heap); +extern VEC(attribute_pair_p,heap) *function_attribute_list; + /* Input file names. */ extern const char **in_fnames; Index: gcc/common.opt =================================================================== --- gcc/common.opt (revision 188050) +++ gcc/common.opt (working copy) @@ -1242,6 +1242,10 @@ Common Report Var(flag_function_sections) Place each function into its own section +ffunction-attribute-list= +Common Joined RejectNegative Var(common_deferred_options) Defer +-ffunction-attribute-list=attribute:name,... Add attribute to named functions + fgcda= Common Joined RejectNegative Var(gcov_da_name) Set the gcov data file name. Index: gcc/opts-global.c =================================================================== --- gcc/opts-global.c (revision 188050) +++ gcc/opts-global.c (working copy) @@ -50,6 +50,8 @@ const char **in_fnames; unsigned num_in_fnames; +VEC(attribute_pair_p,heap) *function_attribute_list; + /* Return a malloced slash-separated list of languages in MASK. */ static char * @@ -79,6 +81,66 @@ return result; } +/* Add strings like attribute_str:name1,name2... to a char_pair_p vector. */ + +static void +add_attribute_list_to_vector (void **pvec, const char *arg) +{ + char *tmp; + char *r; + char *w; + char *token_start; + char *attribute; + VEC(attribute_pair_p,heap) *vec = (VEC(attribute_pair_p,heap) *) *pvec; + + /* We never free this string. */ + tmp = xstrdup (arg); + attribute = tmp; + + for (r = tmp; *r != '\0' && *r != ':'; ++r) + ; + + if (*r != ':') + return; + + *r = '\0'; + ++r; + w = r; + token_start = r; + + while (*r != '\0') + { + if (*r == ',') + { + attribute_pair_p p = + (attribute_pair_p) xmalloc (sizeof (attribute_pair)); + p->name = token_start; + p->attribute = attribute; + *w++ = '\0'; + ++r; + VEC_safe_push (attribute_pair_p, heap, vec, p); + token_start = w; + } + if (*r == '\\' && r[1] == ',') + { + *w++ = ','; + r += 2; + } + else + *w++ = *r++; + } + if (*token_start != '\0') + { + attribute_pair_p p = + (attribute_pair_p) xmalloc (sizeof (attribute_pair)); + p->name = token_start; + p->attribute = attribute; + VEC_safe_push (attribute_pair_p, heap, vec, p); + } + + *pvec = vec; +} + /* Complain that switch DECODED does not apply to this front end (mask LANG_MASK). */ @@ -452,6 +514,10 @@ set_random_seed (opt->arg); break; + case OPT_ffunction_attribute_list_: + add_attribute_list_to_vector(&function_attribute_list, opt->arg); + break; + case OPT_fstack_limit: /* The real switch is -fno-stack-limit. */ if (!opt->value)