| Message ID | CAEe8uEDfSVdv6-XrxCKkej0GYh-j9iA3oXOT3Gfwq73bYCOF9g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 2, 2013 at 10:41 PM, Carrot Wei <carrot@google.com> wrote: > This patch outline the construction of gcov constructor from coverage_obj_init > as a separate function build_init_ctor. > > It passed bootstrap and regression test on x86-64. > > OK for trunk and google 4.7 branch? Please pass gcov_info_type as parameter to build_init_ctor to avoid the new GC root. Ok with that change. Thanks, Richard. > thanks > Carrot > > > 2013-05-02 Guozhi Wei <carrot@google.com> > > * coverage.c (gcov_info_type): New global variable. > (coverage_obj_init): Move the construction of gcov constructor to > (build_init_ctor): here. > > > Index: coverage.c > =================================================================== > --- coverage.c (revision 198557) > +++ coverage.c (working copy) > @@ -99,6 +99,7 @@ > > /* Coverage info VAR_DECL and function info type nodes. */ > static GTY(()) tree gcov_info_var; > +static GTY(()) tree gcov_info_type; > static GTY(()) tree gcov_fn_info_type; > static GTY(()) tree gcov_fn_info_ptr_type; > > @@ -967,6 +968,32 @@ > return build_constructor (info_type, v1); > } > > +/* Generate the constructor function to call __gcov_init. */ > + > +static void > +build_init_ctor () > +{ > + tree ctor, stmt, init_fn; > + > + /* Build a decl for __gcov_init. */ > + init_fn = build_pointer_type (gcov_info_type); > + init_fn = build_function_type_list (void_type_node, init_fn, NULL); > + init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, > + get_identifier ("__gcov_init"), init_fn); > + TREE_PUBLIC (init_fn) = 1; > + DECL_EXTERNAL (init_fn) = 1; > + DECL_ASSEMBLER_NAME (init_fn); > + > + /* Generate a call to __gcov_init(&gcov_info). */ > + ctor = NULL; > + stmt = build_fold_addr_expr (gcov_info_var); > + stmt = build_call_expr (init_fn, 1, stmt); > + append_to_statement_list (stmt, &ctor); > + > + /* Generate a constructor to run it. */ > + cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); > +} > + > /* Create the gcov_info types and object. Generate the constructor > function to call __gcov_init. Does not generate the initializer > for the object. Returns TRUE if coverage data is being emitted. */ > @@ -974,7 +1001,6 @@ > static bool > coverage_obj_init (void) > { > - tree gcov_info_type, ctor, stmt, init_fn; > unsigned n_counters = 0; > unsigned ix; > struct coverage_data *fn; > @@ -1020,24 +1046,8 @@ > ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0); > DECL_NAME (gcov_info_var) = get_identifier (name_buf); > > - /* Build a decl for __gcov_init. */ > - init_fn = build_pointer_type (gcov_info_type); > - init_fn = build_function_type_list (void_type_node, init_fn, NULL); > - init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, > - get_identifier ("__gcov_init"), init_fn); > - TREE_PUBLIC (init_fn) = 1; > - DECL_EXTERNAL (init_fn) = 1; > - DECL_ASSEMBLER_NAME (init_fn); > + build_init_ctor (); > > - /* Generate a call to __gcov_init(&gcov_info). */ > - ctor = NULL; > - stmt = build_fold_addr_expr (gcov_info_var); > - stmt = build_call_expr (init_fn, 1, stmt); > - append_to_statement_list (stmt, &ctor); > - > - /* Generate a constructor to run it. */ > - cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); > - > return true; > }
On Fri, May 3, 2013 at 1:03 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, May 2, 2013 at 10:41 PM, Carrot Wei <carrot@google.com> wrote: >> This patch outline the construction of gcov constructor from coverage_obj_init >> as a separate function build_init_ctor. >> >> It passed bootstrap and regression test on x86-64. >> >> OK for trunk and google 4.7 branch? > > Please pass gcov_info_type as parameter to build_init_ctor to avoid > the new GC root. Pass gcov_info_type as parameter to build_init_ctor may look more cleaner, but it may limit build_init_ctor can only be called from coverage_obj_init. In some situations build_init_ctor may need to be called by other functions, such as my patch for lipo http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00078.html. On the other hand, gcov_info_type is the only non-global variable in the family of variables that have similar purpose. So it is not very bad to make it also global. thanks Carrot > > Ok with that change. > > Thanks, > Richard. > >> thanks >> Carrot >> >> >> 2013-05-02 Guozhi Wei <carrot@google.com> >> >> * coverage.c (gcov_info_type): New global variable. >> (coverage_obj_init): Move the construction of gcov constructor to >> (build_init_ctor): here. >> >> >> Index: coverage.c >> =================================================================== >> --- coverage.c (revision 198557) >> +++ coverage.c (working copy) >> @@ -99,6 +99,7 @@ >> >> /* Coverage info VAR_DECL and function info type nodes. */ >> static GTY(()) tree gcov_info_var; >> +static GTY(()) tree gcov_info_type; >> static GTY(()) tree gcov_fn_info_type; >> static GTY(()) tree gcov_fn_info_ptr_type; >> >> @@ -967,6 +968,32 @@ >> return build_constructor (info_type, v1); >> } >> >> +/* Generate the constructor function to call __gcov_init. */ >> + >> +static void >> +build_init_ctor () >> +{ >> + tree ctor, stmt, init_fn; >> + >> + /* Build a decl for __gcov_init. */ >> + init_fn = build_pointer_type (gcov_info_type); >> + init_fn = build_function_type_list (void_type_node, init_fn, NULL); >> + init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, >> + get_identifier ("__gcov_init"), init_fn); >> + TREE_PUBLIC (init_fn) = 1; >> + DECL_EXTERNAL (init_fn) = 1; >> + DECL_ASSEMBLER_NAME (init_fn); >> + >> + /* Generate a call to __gcov_init(&gcov_info). */ >> + ctor = NULL; >> + stmt = build_fold_addr_expr (gcov_info_var); >> + stmt = build_call_expr (init_fn, 1, stmt); >> + append_to_statement_list (stmt, &ctor); >> + >> + /* Generate a constructor to run it. */ >> + cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); >> +} >> + >> /* Create the gcov_info types and object. Generate the constructor >> function to call __gcov_init. Does not generate the initializer >> for the object. Returns TRUE if coverage data is being emitted. */ >> @@ -974,7 +1001,6 @@ >> static bool >> coverage_obj_init (void) >> { >> - tree gcov_info_type, ctor, stmt, init_fn; >> unsigned n_counters = 0; >> unsigned ix; >> struct coverage_data *fn; >> @@ -1020,24 +1046,8 @@ >> ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0); >> DECL_NAME (gcov_info_var) = get_identifier (name_buf); >> >> - /* Build a decl for __gcov_init. */ >> - init_fn = build_pointer_type (gcov_info_type); >> - init_fn = build_function_type_list (void_type_node, init_fn, NULL); >> - init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, >> - get_identifier ("__gcov_init"), init_fn); >> - TREE_PUBLIC (init_fn) = 1; >> - DECL_EXTERNAL (init_fn) = 1; >> - DECL_ASSEMBLER_NAME (init_fn); >> + build_init_ctor (); >> >> - /* Generate a call to __gcov_init(&gcov_info). */ >> - ctor = NULL; >> - stmt = build_fold_addr_expr (gcov_info_var); >> - stmt = build_call_expr (init_fn, 1, stmt); >> - append_to_statement_list (stmt, &ctor); >> - >> - /* Generate a constructor to run it. */ >> - cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); >> - >> return true; >> }
Please do what Richard suggested. gcov_info_type can be obtained from gcov_info_var decl. David On Fri, May 3, 2013 at 11:31 AM, Carrot Wei <carrot@google.com> wrote: > On Fri, May 3, 2013 at 1:03 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, May 2, 2013 at 10:41 PM, Carrot Wei <carrot@google.com> wrote: >>> This patch outline the construction of gcov constructor from coverage_obj_init >>> as a separate function build_init_ctor. >>> >>> It passed bootstrap and regression test on x86-64. >>> >>> OK for trunk and google 4.7 branch? >> >> Please pass gcov_info_type as parameter to build_init_ctor to avoid >> the new GC root. > > Pass gcov_info_type as parameter to build_init_ctor may look more cleaner, > but it may limit build_init_ctor can only be called from coverage_obj_init. > In some situations build_init_ctor may need to be called by other functions, > such as my patch for lipo > http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00078.html. > > On the other hand, gcov_info_type is the only non-global variable in the > family of variables that have similar purpose. So it is not very bad to make > it also global. > > thanks > Carrot > >> >> Ok with that change. >> >> Thanks, >> Richard. >> >>> thanks >>> Carrot >>> >>> >>> 2013-05-02 Guozhi Wei <carrot@google.com> >>> >>> * coverage.c (gcov_info_type): New global variable. >>> (coverage_obj_init): Move the construction of gcov constructor to >>> (build_init_ctor): here. >>> >>> >>> Index: coverage.c >>> =================================================================== >>> --- coverage.c (revision 198557) >>> +++ coverage.c (working copy) >>> @@ -99,6 +99,7 @@ >>> >>> /* Coverage info VAR_DECL and function info type nodes. */ >>> static GTY(()) tree gcov_info_var; >>> +static GTY(()) tree gcov_info_type; >>> static GTY(()) tree gcov_fn_info_type; >>> static GTY(()) tree gcov_fn_info_ptr_type; >>> >>> @@ -967,6 +968,32 @@ >>> return build_constructor (info_type, v1); >>> } >>> >>> +/* Generate the constructor function to call __gcov_init. */ >>> + >>> +static void >>> +build_init_ctor () >>> +{ >>> + tree ctor, stmt, init_fn; >>> + >>> + /* Build a decl for __gcov_init. */ >>> + init_fn = build_pointer_type (gcov_info_type); >>> + init_fn = build_function_type_list (void_type_node, init_fn, NULL); >>> + init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, >>> + get_identifier ("__gcov_init"), init_fn); >>> + TREE_PUBLIC (init_fn) = 1; >>> + DECL_EXTERNAL (init_fn) = 1; >>> + DECL_ASSEMBLER_NAME (init_fn); >>> + >>> + /* Generate a call to __gcov_init(&gcov_info). */ >>> + ctor = NULL; >>> + stmt = build_fold_addr_expr (gcov_info_var); >>> + stmt = build_call_expr (init_fn, 1, stmt); >>> + append_to_statement_list (stmt, &ctor); >>> + >>> + /* Generate a constructor to run it. */ >>> + cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); >>> +} >>> + >>> /* Create the gcov_info types and object. Generate the constructor >>> function to call __gcov_init. Does not generate the initializer >>> for the object. Returns TRUE if coverage data is being emitted. */ >>> @@ -974,7 +1001,6 @@ >>> static bool >>> coverage_obj_init (void) >>> { >>> - tree gcov_info_type, ctor, stmt, init_fn; >>> unsigned n_counters = 0; >>> unsigned ix; >>> struct coverage_data *fn; >>> @@ -1020,24 +1046,8 @@ >>> ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0); >>> DECL_NAME (gcov_info_var) = get_identifier (name_buf); >>> >>> - /* Build a decl for __gcov_init. */ >>> - init_fn = build_pointer_type (gcov_info_type); >>> - init_fn = build_function_type_list (void_type_node, init_fn, NULL); >>> - init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, >>> - get_identifier ("__gcov_init"), init_fn); >>> - TREE_PUBLIC (init_fn) = 1; >>> - DECL_EXTERNAL (init_fn) = 1; >>> - DECL_ASSEMBLER_NAME (init_fn); >>> + build_init_ctor (); >>> >>> - /* Generate a call to __gcov_init(&gcov_info). */ >>> - ctor = NULL; >>> - stmt = build_fold_addr_expr (gcov_info_var); >>> - stmt = build_call_expr (init_fn, 1, stmt); >>> - append_to_statement_list (stmt, &ctor); >>> - >>> - /* Generate a constructor to run it. */ >>> - cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); >>> - >>> return true; >>> }
commited as 198591. On Fri, May 3, 2013 at 11:51 AM, Xinliang David Li <davidxl@google.com> wrote: > Please do what Richard suggested. gcov_info_type can be obtained from > gcov_info_var decl. > > David > > > On Fri, May 3, 2013 at 11:31 AM, Carrot Wei <carrot@google.com> wrote: >> On Fri, May 3, 2013 at 1:03 AM, Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Thu, May 2, 2013 at 10:41 PM, Carrot Wei <carrot@google.com> wrote: >>>> This patch outline the construction of gcov constructor from coverage_obj_init >>>> as a separate function build_init_ctor. >>>> >>>> It passed bootstrap and regression test on x86-64. >>>> >>>> OK for trunk and google 4.7 branch? >>> >>> Please pass gcov_info_type as parameter to build_init_ctor to avoid >>> the new GC root. >> >> Pass gcov_info_type as parameter to build_init_ctor may look more cleaner, >> but it may limit build_init_ctor can only be called from coverage_obj_init. >> In some situations build_init_ctor may need to be called by other functions, >> such as my patch for lipo >> http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00078.html. >> >> On the other hand, gcov_info_type is the only non-global variable in the >> family of variables that have similar purpose. So it is not very bad to make >> it also global. >> >> thanks >> Carrot >> >>> >>> Ok with that change. >>> >>> Thanks, >>> Richard. >>>
Index: coverage.c =================================================================== --- coverage.c (revision 198557) +++ coverage.c (working copy) @@ -99,6 +99,7 @@ /* Coverage info VAR_DECL and function info type nodes. */ static GTY(()) tree gcov_info_var; +static GTY(()) tree gcov_info_type; static GTY(()) tree gcov_fn_info_type; static GTY(()) tree gcov_fn_info_ptr_type; @@ -967,6 +968,32 @@ return build_constructor (info_type, v1); } +/* Generate the constructor function to call __gcov_init. */ + +static void +build_init_ctor () +{ + tree ctor, stmt, init_fn; + + /* Build a decl for __gcov_init. */ + init_fn = build_pointer_type (gcov_info_type); + init_fn = build_function_type_list (void_type_node, init_fn, NULL); + init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, + get_identifier ("__gcov_init"), init_fn); + TREE_PUBLIC (init_fn) = 1; + DECL_EXTERNAL (init_fn) = 1; + DECL_ASSEMBLER_NAME (init_fn); + + /* Generate a call to __gcov_init(&gcov_info). */ + ctor = NULL; + stmt = build_fold_addr_expr (gcov_info_var); + stmt = build_call_expr (init_fn, 1, stmt); + append_to_statement_list (stmt, &ctor); + + /* Generate a constructor to run it. */ + cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); +} + /* Create the gcov_info types and object. Generate the constructor function to call __gcov_init. Does not generate the initializer for the object. Returns TRUE if coverage data is being emitted. */ @@ -974,7 +1001,6 @@ static bool coverage_obj_init (void) { - tree gcov_info_type, ctor, stmt, init_fn; unsigned n_counters = 0; unsigned ix; struct coverage_data *fn; @@ -1020,24 +1046,8 @@ ASM_GENERATE_INTERNAL_LABEL (name_buf, "LPBX", 0); DECL_NAME (gcov_info_var) = get_identifier (name_buf); - /* Build a decl for __gcov_init. */ - init_fn = build_pointer_type (gcov_info_type); - init_fn = build_function_type_list (void_type_node, init_fn, NULL); - init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL, - get_identifier ("__gcov_init"), init_fn); - TREE_PUBLIC (init_fn) = 1; - DECL_EXTERNAL (init_fn) = 1; - DECL_ASSEMBLER_NAME (init_fn); + build_init_ctor (); - /* Generate a call to __gcov_init(&gcov_info). */ - ctor = NULL; - stmt = build_fold_addr_expr (gcov_info_var); - stmt = build_call_expr (init_fn, 1, stmt); - append_to_statement_list (stmt, &ctor); - - /* Generate a constructor to run it. */ - cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY); - return true; }
This patch outline the construction of gcov constructor from coverage_obj_init as a separate function build_init_ctor. It passed bootstrap and regression test on x86-64. OK for trunk and google 4.7 branch? thanks Carrot 2013-05-02 Guozhi Wei <carrot@google.com> * coverage.c (gcov_info_type): New global variable. (coverage_obj_init): Move the construction of gcov constructor to (build_init_ctor): here.