Hi,

when testing the experimental hsa branch, where dynamic parallelism is
not disabled and get_hsa_kernel_dispatch_offset is executed quite a
bit more frequently, I have come across hsa_kernel_dispatch_type being
freed by gcc even though it is marked with a GTY flag.  The reason is
that the file hsa-gen.c is not listed in GTFILES in Makefile.in (and
this it does not have and does not include its gcc header file).  This
was the only intended GTY root in this file but there is another one
in hsa-brig.c for lists of statements to put into a static
constructors and destructors.

Rather than adding these files to GTFILES, I have decided to move the
tree roots to hsa.c which is already there and GTY roots are really
only needed for these few rather special occasions.  The patch below,
which does just that, passed bootstrap and testing and HSA testing on
both trunk and the branch.  I will commit it in a few moments.

Thanks,

Martin


2016-03-02  Martin Jambor  <mjam...@suse.cz>

        * hsa.h (hsa_get_ctor_statements): Declare.
        (hsa_get_dtor_statements): Likewise.
        (hsa_get_kernel_dispatch_type): Likewise.
        * hsa.c (hsa_get_ctor_statements): New function.
        (hsa_get_dtor_statements): Likewise.
        (hsa_get_kernel_dispatch_type): Likewise.
        * hsa-brig.c (hsa_cdtor_statements): Removed.
        (hsa_output_libgomp_mapping): Use hsa_get_ctor_statements and
        hsa_get_dtor_statements.
        * hsa-gen.c (hsa_kernel_dispatch_type): Removed.
        (get_hsa_kernel_dispatch_offset): Use hsa_get_kernel_dispatch_type.
---
 gcc/hsa-brig.c | 14 ++++++--------
 gcc/hsa-gen.c  | 13 ++++++-------
 gcc/hsa.c      | 25 +++++++++++++++++++++++++
 gcc/hsa.h      |  3 +++
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/gcc/hsa-brig.c b/gcc/hsa-brig.c
index 61cfd8b..2a301be 100644
--- a/gcc/hsa-brig.c
+++ b/gcc/hsa-brig.c
@@ -2006,8 +2006,6 @@ hsa_brig_emit_omp_symbols (void)
   emit_directive_variable (hsa_num_threads);
 }
 
-static GTY(()) tree hsa_cdtor_statements[2];
-
 /* Create and return __hsa_global_variables symbol that contains
    all informations consumed by libgomp to link global variables
    with their string names used by an HSA kernel.  */
@@ -2408,6 +2406,7 @@ hsa_output_libgomp_mapping (tree brig_decl)
     = builtin_decl_explicit (BUILT_IN_GOMP_OFFLOAD_REGISTER);
   gcc_checking_assert (offload_register);
 
+  tree *hsa_ctor_stmts = hsa_get_ctor_statements ();
   append_to_statement_list
     (build_call_expr (offload_register, 4,
                      build_int_cstu (unsigned_type_node,
@@ -2416,15 +2415,15 @@ hsa_output_libgomp_mapping (tree brig_decl)
                      build_fold_addr_expr (hsa_libgomp_host_table),
                      build_int_cst (integer_type_node, GOMP_DEVICE_HSA),
                      build_fold_addr_expr (hsa_img_descriptor)),
-     &hsa_cdtor_statements[0]);
+     hsa_ctor_stmts);
 
-  cgraph_build_static_cdtor ('I', hsa_cdtor_statements[0],
-                            DEFAULT_INIT_PRIORITY);
+  cgraph_build_static_cdtor ('I', *hsa_ctor_stmts, DEFAULT_INIT_PRIORITY);
 
   tree offload_unregister
     = builtin_decl_explicit (BUILT_IN_GOMP_OFFLOAD_UNREGISTER);
   gcc_checking_assert (offload_unregister);
 
+  tree *hsa_dtor_stmts = hsa_get_dtor_statements ();
   append_to_statement_list
     (build_call_expr (offload_unregister, 4,
                      build_int_cstu (unsigned_type_node,
@@ -2433,9 +2432,8 @@ hsa_output_libgomp_mapping (tree brig_decl)
                      build_fold_addr_expr (hsa_libgomp_host_table),
                      build_int_cst (integer_type_node, GOMP_DEVICE_HSA),
                      build_fold_addr_expr (hsa_img_descriptor)),
-     &hsa_cdtor_statements[1]);
-  cgraph_build_static_cdtor ('D', hsa_cdtor_statements[1],
-                            DEFAULT_INIT_PRIORITY);
+     hsa_dtor_stmts);
+  cgraph_build_static_cdtor ('D', *hsa_dtor_stmts, DEFAULT_INIT_PRIORITY);
 }
 
 /* Emit the brig module we have compiled to a section in the final assembly and
diff --git a/gcc/hsa-gen.c b/gcc/hsa-gen.c
index d7d39f0..fc59fa5 100644
--- a/gcc/hsa-gen.c
+++ b/gcc/hsa-gen.c
@@ -3772,20 +3772,19 @@ gen_set_num_threads (tree value, hsa_bb *hbb)
   hbb->append_insn (basic);
 }
 
-static GTY (()) tree hsa_kernel_dispatch_type = NULL;
-
 /* Return byte offset of a FIELD_NAME in GOMP_hsa_kernel_dispatch which
    is defined in plugin-hsa.c.  */
 
 static HOST_WIDE_INT
 get_hsa_kernel_dispatch_offset (const char *field_name)
 {
-  if (hsa_kernel_dispatch_type == NULL)
+  tree *hsa_kernel_dispatch_type = hsa_get_kernel_dispatch_type ();
+  if (*hsa_kernel_dispatch_type == NULL)
     {
       /* Collection of information needed for a dispatch of a kernel from a
         kernel.  Keep in sync with libgomp's plugin-hsa.c.  */
 
-      hsa_kernel_dispatch_type = make_node (RECORD_TYPE);
+      *hsa_kernel_dispatch_type = make_node (RECORD_TYPE);
       tree id_f1 = build_decl (BUILTINS_LOCATION, FIELD_DECL,
                               get_identifier ("queue"), ptr_type_node);
       DECL_CHAIN (id_f1) = NULL_TREE;
@@ -3835,12 +3834,12 @@ get_hsa_kernel_dispatch_offset (const char *field_name)
       DECL_CHAIN (id_f12) = id_f11;
 
 
-      finish_builtin_struct (hsa_kernel_dispatch_type, "__hsa_kernel_dispatch",
+      finish_builtin_struct (*hsa_kernel_dispatch_type, 
"__hsa_kernel_dispatch",
                             id_f12, NULL_TREE);
-      TYPE_ARTIFICIAL (hsa_kernel_dispatch_type) = 1;
+      TYPE_ARTIFICIAL (*hsa_kernel_dispatch_type) = 1;
     }
 
-  for (tree chain = TYPE_FIELDS (hsa_kernel_dispatch_type);
+  for (tree chain = TYPE_FIELDS (*hsa_kernel_dispatch_type);
        chain != NULL_TREE; chain = TREE_CHAIN (chain))
     if (strcmp (field_name, IDENTIFIER_POINTER (DECL_NAME (chain))) == 0)
       return int_byte_position (chain);
diff --git a/gcc/hsa.c b/gcc/hsa.c
index 09bfd28..9eacb59 100644
--- a/gcc/hsa.c
+++ b/gcc/hsa.c
@@ -712,6 +712,31 @@ hsa_add_kernel_dependency (tree caller, const char 
*called_function)
   s->safe_push (called_function);
 }
 
+/* Expansion to HSA needs a few gc roots to hold types, constructors etc.  In
+   order to minimize the number of GTY roots, we'll root them all in the
+   following array.  The individual elements should only be accessed by the
+   very simple getters (of a pointer-to-tree) below.  */
+
+static GTY(()) tree hsa_tree_gt_roots[3];
+
+tree *
+hsa_get_ctor_statements (void)
+{
+  return &hsa_tree_gt_roots[0];
+}
+
+tree *
+hsa_get_dtor_statements (void)
+{
+  return &hsa_tree_gt_roots[1];
+}
+
+tree *
+hsa_get_kernel_dispatch_type (void)
+{
+  return &hsa_tree_gt_roots[2];
+}
+
 /* Modify the name P in-place so that it is a valid HSA identifier.  */
 
 void
diff --git a/gcc/hsa.h b/gcc/hsa.h
index 3a13fbd..6a7c651 100644
--- a/gcc/hsa.h
+++ b/gcc/hsa.h
@@ -1353,6 +1353,9 @@ char *hsa_get_decl_kernel_mapping_name (unsigned i);
 unsigned hsa_get_decl_kernel_mapping_omp_size (unsigned i);
 bool hsa_get_decl_kernel_mapping_gridified (unsigned i);
 void hsa_free_decl_kernel_mapping (void);
+tree *hsa_get_ctor_statements (void);
+tree *hsa_get_dtor_statements (void);
+tree *hsa_get_kernel_dispatch_type (void);
 void hsa_add_kernel_dependency (tree caller, const char *called_function);
 void hsa_sanitize_name (char *p);
 char *hsa_brig_function_name (const char *p);
-- 
2.7.1

Reply via email to