diff mbox

[google,4.7] Move the building of gcov constructor function after initialization of gcov_info_var

Message ID CAEe8uEAuW_PgrUKo8tZuY7Kn4vcVQOuMPp7_1tGaz7dWvQ_5vQ@mail.gmail.com
State New
Headers show

Commit Message

Carrot Wei May 2, 2013, 6:06 p.m. UTC
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  <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.
(coverage_obj_finish): Call build_init_ctor after initialization of
gcov_info_var.

Comments

Xinliang David Li May 2, 2013, 6:15 p.m. UTC | #1
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 <carrot@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  <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.
> (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);
>  }
diff mbox

Patch

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);
 }