On Fri, May 3, 2013 at 1:03 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Thu, May 2, 2013 at 10:41 PM, Carrot Wei <car...@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  <car...@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;
>>  }

Reply via email to