I suggest submitting the refactoring part of the changes to GCC trunk first.
thanks,
David
On Thu, May 2, 2013 at 11:06 AM, Carrot Wei <car...@google.com> wrote:
> This patch fixes google bug 8397853 and targets google 4.7 branch.
>
> In LIPO mode, when coverage_obj_init is called, cgraph_state is
> CGRAPH_STATE_FINISHED. The variable gcov_info_var is created but not
> initialized. When cgraph_build_static_cdtor is called, the new function and
> variables are expanded immediately since cgraph_state is
> CGRAPH_STATE_FINISHED.
> It causes gcov_info_var into .bss section. But later in function
> coverage_obj_finish we initialize gcov_info_var with non zero contents, so it
> should not be put into .bss section.
>
> In FDO mode we don't have this problem because when coverage_obj_init is
> called,
> cgraph_state is CGRAPH_STATE_IPA_SSA. When cgraph_build_static_cdtor is
> called,
> the new function is not immediately expanded. The variable will have been
> properly initialized when it is expanded.
>
> It can be fixed by moving the construction of gcov constructor after
> initialization of gcov_info_var.
>
> Tested with following testing:
> x86-64 bootstrap.
> x86-64 regression test.
> power64 regression test on qemu.
>
> The only regression for power64 is
> FAIL: gcc.dg/torture/tls/tls-test.c -O2 -flto -fno-use-linker-plugin
> -flto-partition=none execution test
> It is a flaky test case in our testing environment since all other executions
> with different compiler options failed. All testing of tls-test.c pass native
> power64 testing.
>
> 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.
> (coverage_obj_finish): Call build_init_ctor after initialization of
> gcov_info_var.
>
>
> Index: coverage.c
> ===================================================================
> --- coverage.c (revision 198425)
> +++ coverage.c (working copy)
> @@ -123,6 +123,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;
>
> @@ -2478,14 +2479,12 @@
> return build_constructor (info_type, v1);
> }
>
> -/* Create the gcov_info types and object. Generate the constructor
> - function to call __gcov_init. Does not generate the initializer
> +/* Create the gcov_info types and object. Does not generate the initializer
> for the object. Returns TRUE if coverage data is being emitted. */
>
> static bool
> coverage_obj_init (void)
> {
> - tree gcov_info_type, ctor, stmt, init_fn;
> unsigned n_counters = 0;
> unsigned ix;
> struct coverage_data *fn;
> @@ -2531,24 +2530,6 @@
> 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);
> -
> - /* 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;
> }
>
> @@ -2570,6 +2551,32 @@
> return ctor;
> }
>
> +/* 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);
> +}
> +
> /* Finalize the coverage data. Generates the array of pointers to
> function objects from CTOR. Generate the gcov_info initializer. */
>
> @@ -2589,9 +2596,12 @@
> DECL_NAME (fn_info_ary) = get_identifier (name_buf);
> DECL_INITIAL (fn_info_ary) = build_constructor (fn_info_ary_type, ctor);
> varpool_finalize_decl (fn_info_ary);
> -
> +
> DECL_INITIAL (gcov_info_var)
> = build_info (TREE_TYPE (gcov_info_var), fn_info_ary);
> +
> + build_init_ctor ();
> +
> varpool_finalize_decl (gcov_info_var);
> }
