On 08/12/2020 4:24 pm, Jakub Jelinek wrote:
The GCC coding style (appart from libstdc++) is type * rather than type*,
occurs several times in the patch.

Fixed.

+{
+  tree node;
+
+  if (DECL_INITIAL (decl))
+    return &DECL_INITIAL (decl);
+
+  for (node = dynamic_initializers; node; node = TREE_CHAIN (node))
+    if (TREE_VALUE (node) == decl)
+      return &TREE_PURPOSE (node);

I'm worried with many dynamic initializers this will be worst case
quadratic.  Can't you use instead a hash map?  Note, as this is in the
FE, we might need to worry about PCH and GC.
Thus the hash map needs to be indexed by DECL_UIDs rather than pointers,
so perhaps use decl_tree_map?
Also, I'm worried that nothing releases dynamic_initializers (or the
decl_tree_map replacement).  We need it only during the discovery and not
afterwards, so it would be nice if the omp declare target discovery at the
end called another lang hook that would free the decl_tree_map, so that GC
can take it all.
If trees would remain there afterwards, we'd need to worry about destructive
gimplifier too and would need to unshare the dynamic initializers or
something.

I think it would be best to use omp_ in the hook name(s), and:

I have now changed dynamic_initializers to use a decl_tree_map instead. get_decl_init has been renamed to omp_get_decl_init, and I have added a hook omp_finish_decl_inits which is called at the end of omp_discover_implicit_declare_target to free the decl_tree_map for GC.

--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4940,6 +4940,11 @@ c_parse_final_cleanups (void)
         loop.  */
        vars = prune_vars_needing_no_initialization (&static_aggregates);
+ /* Copy the contents of VARS into DYNAMIC_INITIALIZERS. */
+      for (t = vars; t; t = TREE_CHAIN (t))
+       dynamic_initializers = tree_cons (TREE_PURPOSE (t), TREE_VALUE (t),
+                                         dynamic_initializers);

Not to add there anything if (!flag_openmp).  We don't need to waste memory
when nobody is going to look at it.

Done.

I have retested all the gomp tests in the main testsuite, retested libgomp, and checked bootstrapping. Is this version okay for trunk now?

Thanks

Kwok
From ef4a42c5174372dd0d72dc0efe2c608e693c7877 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung <k...@codesourcery.com>
Date: Thu, 17 Dec 2020 12:10:18 -0800
Subject: [PATCH] openmp: Implicitly add 'declare target' directives for
 dynamic initializers in C++

2020-12-17  Kwok Cheung Yeung  <k...@codesourcery.com>

        gcc/
        * langhooks-def.h (lhd_get_decl_init): New.
        (lhd_finish_decl_inits): New.
        (LANG_HOOKS_GET_DECL_INIT): New.
        (LANG_HOOKS_OMP_FINISH_DECL_INITS): New.
        (LANG_HOOKS_DECLS): Add LANG_HOOKS_GET_DECL_INIT and
        LANG_HOOKS_OMP_FINISH_DECL_INITS.
        * langhooks.c (lhd_omp_get_decl_init): New.
        (lhd_omp_finish_decl_inits): New.
        * langhooks.h (struct lang_hooks_for_decls): Add omp_get_decl_init
        and omp_finish_decl_inits.
        * omp-offload.c (omp_discover_declare_target_var_r): Use
        get_decl_init langhook in place of DECL_INITIAL.  Call
        omp_finish_decl_inits langhook at end of function.

        gcc/cp/
        * cp-lang.c (cxx_get_decl_init): New.
        (cxx_omp_finish_decl_inits): New.
        (LANG_HOOKS_GET_DECL_INIT): New.
        (LANG_HOOKS_OMP_FINISH_DECL_INITS): New.
        * cp-tree.h (dynamic_initializers): New.
        * decl.c (dynamic_initializers): New.
        * decl2.c (c_parse_final_cleanups): Add initializer entries
        from vars to dynamic_initializers.

        gcc/testsuite/
        * g++.dg/gomp/declare-target-3.C: New.
---
 gcc/cp/cp-lang.c                             | 32 ++++++++++++++++++++++++++++
 gcc/cp/cp-tree.h                             |  4 ++++
 gcc/cp/decl.c                                |  4 ++++
 gcc/cp/decl2.c                               |  7 ++++++
 gcc/langhooks-def.h                          |  8 ++++++-
 gcc/langhooks.c                              | 16 ++++++++++++++
 gcc/langhooks.h                              | 10 +++++++++
 gcc/omp-offload.c                            | 11 ++++++----
 gcc/testsuite/g++.dg/gomp/declare-target-3.C | 31 +++++++++++++++++++++++++++
 9 files changed, 118 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/gomp/declare-target-3.C

diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index 5d2aef4..bde11db 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -34,6 +34,8 @@ static tree cp_eh_personality (void);
 static tree get_template_innermost_arguments_folded (const_tree);
 static tree get_template_argument_pack_elems_folded (const_tree);
 static tree cxx_enum_underlying_base_type (const_tree);
+static tree * cxx_omp_get_decl_init (tree);
+static void cxx_omp_finish_decl_inits (void);
 
 /* Lang hooks common to C++ and ObjC++ are declared in cp/cp-objcp-common.h;
    consequently, there should be very few hooks below.  */
@@ -92,6 +94,12 @@ static tree cxx_enum_underlying_base_type (const_tree);
 #undef LANG_HOOKS_GET_SUBSTRING_LOCATION
 #define LANG_HOOKS_GET_SUBSTRING_LOCATION c_get_substring_location
 
+#undef LANG_HOOKS_OMP_GET_DECL_INIT
+#define LANG_HOOKS_OMP_GET_DECL_INIT cxx_omp_get_decl_init
+
+#undef LANG_HOOKS_OMP_FINISH_DECL_INITS
+#define LANG_HOOKS_OMP_FINISH_DECL_INITS cxx_omp_finish_decl_inits
+
 /* Each front end provides its own lang hook initializer.  */
 struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
 
@@ -233,6 +241,30 @@ tree cxx_enum_underlying_base_type (const_tree type)
   return underlying_type;
 }
 
+/* The C++ version of the omp_get_decl_init langhook returns the static
+   initializer for a variable declaration if present, otherwise it
+   tries to find and return the dynamic initializer.  If not present,
+   it returns NULL.  */
+
+static tree *
+cxx_omp_get_decl_init (tree decl)
+{
+  if (DECL_INITIAL (decl))
+    return &DECL_INITIAL (decl);
+
+  return hash_map_safe_get (dynamic_initializers, decl);
+}
+
+/* The C++ version of the omp_finish_decl_inits langhook allows GC to
+   reclaim the memory used by the hash-map used to hold dynamic initializer
+   information.  */
+
+static void
+cxx_omp_finish_decl_inits (void)
+{
+  dynamic_initializers = NULL;
+}
+
 #if CHECKING_P
 
 namespace selftest {
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ef5baea..edaa594 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5631,6 +5631,10 @@ extern GTY(()) tree static_aggregates;
 /* Likewise, for thread local storage.  */
 extern GTY(()) tree tls_aggregates;
 
+/* A hash-map mapping from variable decls to the dynamic initializer for
+   the decl.  This is currently only used by OpenMP.  */
+extern GTY(()) decl_tree_map *dynamic_initializers;
+
 enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG };
 
 /* These are uses as bits in flags passed to various functions to
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index b56eb11..c349716 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -146,6 +146,10 @@ tree static_aggregates;
 /* Like static_aggregates, but for thread_local variables.  */
 tree tls_aggregates;
 
+/* A hash-map mapping from variable decls to the dynamic initializer for
+   the decl.  This is currently only used by OpenMP.  */
+decl_tree_map *dynamic_initializers;
+
 /* -- end of C++ */
 
 /* A node for the integer constant 2.  */
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 6d8158a..af88e7f 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -5006,6 +5006,13 @@ c_parse_final_cleanups (void)
         loop.  */
       if (tree vars = prune_vars_needing_no_initialization 
(&static_aggregates))
        {
+         if (flag_openmp)
+           /* Add initializer information from VARS into
+              DYNAMIC_INITIALIZERS.  */
+           for (t = vars; t; t = TREE_CHAIN (t))
+             hash_map_safe_put<hm_ggc> (dynamic_initializers,
+                                        TREE_VALUE (t), TREE_PURPOSE (t));
+
          /* We need to start a new initialization function each time
             through the loop.  That's because we need to know which
             vtables have been referenced, and TREE_SYMBOL_REFERENCED
diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h
index 2f66f5e..59178d2 100644
--- a/gcc/langhooks-def.h
+++ b/gcc/langhooks-def.h
@@ -87,6 +87,8 @@ extern void lhd_omp_firstprivatize_type_sizes (struct 
gimplify_omp_ctx *,
                                               tree);
 extern bool lhd_omp_mappable_type (tree);
 extern bool lhd_omp_scalar_p (tree);
+extern tree * lhd_omp_get_decl_init (tree);
+extern void lhd_omp_finish_decl_inits ();
 
 extern const char *lhd_get_substring_location (const substring_loc &,
                                               location_t *out_loc);
@@ -265,6 +267,8 @@ extern tree lhd_unit_size_without_reusable_padding (tree);
 #define LANG_HOOKS_OMP_CLAUSE_DTOR hook_tree_tree_tree_null
 #define LANG_HOOKS_OMP_FINISH_CLAUSE lhd_omp_finish_clause
 #define LANG_HOOKS_OMP_SCALAR_P lhd_omp_scalar_p
+#define LANG_HOOKS_OMP_GET_DECL_INIT lhd_omp_get_decl_init
+#define LANG_HOOKS_OMP_FINISH_DECL_INITS lhd_omp_finish_decl_inits
 
 #define LANG_HOOKS_DECLS { \
   LANG_HOOKS_GLOBAL_BINDINGS_P, \
@@ -293,7 +297,9 @@ extern tree lhd_unit_size_without_reusable_padding (tree);
   LANG_HOOKS_OMP_CLAUSE_LINEAR_CTOR, \
   LANG_HOOKS_OMP_CLAUSE_DTOR, \
   LANG_HOOKS_OMP_FINISH_CLAUSE, \
-  LANG_HOOKS_OMP_SCALAR_P \
+  LANG_HOOKS_OMP_SCALAR_P, \
+  LANG_HOOKS_OMP_GET_DECL_INIT, \
+  LANG_HOOKS_OMP_FINISH_DECL_INITS \
 }
 
 /* LTO hooks.  */
diff --git a/gcc/langhooks.c b/gcc/langhooks.c
index d82f542..23cbf06 100644
--- a/gcc/langhooks.c
+++ b/gcc/langhooks.c
@@ -632,6 +632,22 @@ lhd_omp_scalar_p (tree decl)
   return false;
 }
 
+/* Return static initializer for DECL.  */
+
+tree *
+lhd_omp_get_decl_init (tree decl)
+{
+  return &DECL_INITIAL (decl);
+}
+
+/* Free any extra memory used to hold initializer information for
+   variable declarations.  */
+
+void
+lhd_omp_finish_decl_inits (void)
+{
+}
+
 /* Register language specific type size variables as potentially OpenMP
    firstprivate variables.  */
 
diff --git a/gcc/langhooks.h b/gcc/langhooks.h
index f12589e..6b90794 100644
--- a/gcc/langhooks.h
+++ b/gcc/langhooks.h
@@ -299,6 +299,16 @@ struct lang_hooks_for_decls
   /* Return true if DECL is a scalar variable (for the purpose of
      implicit firstprivatization).  */
   bool (*omp_scalar_p) (tree decl);
+
+  /* Return a pointer to the tree representing the initializer
+     expression for the non-local variable DECL.  Return NULL if
+     DECL is not initialized.  */
+  tree * (*omp_get_decl_init) (tree decl);
+
+  /* Free any extra memory used to hold initializer information for
+     variable declarations.  omp_get_decl_init must not be called
+     after calling this.  */
+  void (*omp_finish_decl_inits) (void);
 };
 
 /* Language hooks related to LTO serialization.  */
diff --git a/gcc/omp-offload.c b/gcc/omp-offload.c
index 9013961..15b735b 100644
--- a/gcc/omp-offload.c
+++ b/gcc/omp-offload.c
@@ -315,7 +315,7 @@ omp_discover_declare_target_var_r (tree *tp, int 
*walk_subtrees, void *data)
          DECL_ATTRIBUTES (*tp)
            = remove_attribute ("omp declare target link", DECL_ATTRIBUTES 
(*tp));
        }
-      if (TREE_STATIC (*tp) && DECL_INITIAL (*tp))
+      if (TREE_STATIC (*tp) && lang_hooks.decls.omp_get_decl_init (*tp))
        ((vec<tree> *) data)->safe_push (*tp);
       DECL_ATTRIBUTES (*tp) = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (*tp));
       symtab_node *node = symtab_node::get (*tp);
@@ -361,14 +361,15 @@ omp_discover_implicit_declare_target (void)
                   && DECL_STRUCT_FUNCTION (cgn->decl)->has_omp_target)
            worklist.safe_push (cgn->decl);
       }
-  FOR_EACH_STATIC_INITIALIZER (vnode)
-    if (omp_declare_target_var_p (vnode->decl))
+  FOR_EACH_VARIABLE (vnode)
+    if (lang_hooks.decls.omp_get_decl_init (vnode->decl)
+       && omp_declare_target_var_p (vnode->decl))
       worklist.safe_push (vnode->decl);
   while (!worklist.is_empty ())
     {
       tree decl = worklist.pop ();
       if (VAR_P (decl))
-       walk_tree_without_duplicates (&DECL_INITIAL (decl),
+       walk_tree_without_duplicates (lang_hooks.decls.omp_get_decl_init (decl),
                                      omp_discover_declare_target_var_r,
                                      &worklist);
       else if (omp_declare_target_fn_p (decl))
@@ -380,6 +381,8 @@ omp_discover_implicit_declare_target (void)
                                      omp_discover_declare_target_fn_r,
                                      &worklist);
     }
+
+  lang_hooks.decls.omp_finish_decl_inits ();
 }
 
 
diff --git a/gcc/testsuite/g++.dg/gomp/declare-target-3.C 
b/gcc/testsuite/g++.dg/gomp/declare-target-3.C
new file mode 100644
index 0000000..d2dedaf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/declare-target-3.C
@@ -0,0 +1,31 @@
+// { dg-do compile }
+// { dg-additional-options "-fdump-tree-gimple" }
+
+// Test implicit marking of declare target to.
+
+int foo () { return 1; }
+int bar () { return 2; }       // Implicitly marked (due to b)
+int baz () { return 3; }       // Implicitly marked (due to d via c)
+int qux () { return 4; }       // Implicitly marked (due to g via f and e)
+
+int a = foo ();
+int b = bar ();        // Explicitly marked
+int c = baz ();        // Implicitly marked (due to d)
+int *d = &c;   // Explicitly marked
+int e = qux ();        // Implicitly marked (due to g via f)
+int f = e + 1; // Implicitly marked (due to g)
+int *g = &f;   // Explicitly marked
+
+#pragma omp declare target to(b, d, g)
+
+// { dg-final { scan-tree-dump-not "__attribute__\\\(\\\(omp declare 
target\\\)\\\)\\\nint foo \\\(\\\)" "gimple" } }
+// { dg-final { scan-tree-dump "__attribute__\\\(\\\(omp declare 
target\\\)\\\)\\\nint bar \\\(\\\)" "gimple" } }
+// { dg-final { scan-tree-dump "__attribute__\\\(\\\(omp declare 
target\\\)\\\)\\\nint baz \\\(\\\)" "gimple" } }
+// { dg-final { scan-tree-dump "__attribute__\\\(\\\(omp declare 
target\\\)\\\)\\\nint qux \\\(\\\)" "gimple" } }
+// { dg-final { scan-assembler-not "\\\.offload_var_table:\\n.+\\\.quad\\s+a" 
} }
+// { dg-final { scan-assembler "\\\.offload_var_table:\\n.+\\\.quad\\s+b" } }
+// { dg-final { scan-assembler "\\\.offload_var_table:\\n.+\\\.quad\\s+c" } }
+// { dg-final { scan-assembler "\\\.offload_var_table:\\n.+\\\.quad\\s+d" } }
+// { dg-final { scan-assembler "\\\.offload_var_table:\\n.+\\\.quad\\s+e" } }
+// { dg-final { scan-assembler "\\\.offload_var_table:\\n.+\\\.quad\\s+f" } }
+// { dg-final { scan-assembler "\\\.offload_var_table:\\n.+\\\.quad\\s+g" } }
-- 
2.8.1

Reply via email to