Patchwork [C++] Attempt to find implicitly determined firstprivate class type vars during genericization (PR c++/48869)

login
register
mail settings
Submitter Jakub Jelinek
Date May 11, 2011, 12:26 p.m.
Message ID <20110511122624.GE17079@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/95245/
State New
Headers show

Comments

Jakub Jelinek - May 11, 2011, 12:26 p.m.
Hi!

Richard's changes to gimplify C++ unit at a time apparently broke
OpenMP 3.0 support in C++, if a variable is implicitly firstprivate
in some task region and needs copy ctor or dtor instantiated, the
copy ctor or dtor might not be instantiated.
In 4.4 when the gimplifier noticed an implicitly firstprivate variable,
it told the FE to finalize the clause and it was instantiated later on,
but now the gimplification happens after instantiation of pending templates.

This patch duplicates parts of the gimplifier's work during genericization,
just to determine if a class typed var referenced in some task region
might be firstprivate or not.  If it finds such vars, it just calls
get_dtor and get_copy_ctor on its type to make sure they will be
instantiated.  I've added to those two functions a COMPLAIN argument,
so that this is done quietly in case the genericization code wasn't 100%
right, the ultimate code is still in the gimplifier, which has to do
such a job anyway e.g. to prevent DECL_VALUE_EXPR vars from expansion
in OpenMP regions when needed, and also so that it is done for all 3 FEs
instead of each FE rolling its own.

Bootstrapped/regtested on x86_64-linux and i686-linux.
Jason, are you ok with it?

2011-05-11  Jakub Jelinek  <jakub@redhat.com>

	PR c++/48869
	* method.c (get_dtor, get_copy_ctor): Add COMPLAIN argument,
	pass it down to locate_fn_flags.
	* cp-tree.h (get_dtor, get_copy_ctor): Adjust prototypes.
	* semantics.c (cxx_omp_create_clause_info): Adjust callers.
	* cp-gimplify.c: Include splay-tree.h.
	(splay_tree_compare_decl_uid, omp_var_to_track,
	omp_cxx_notice_variable): New functions.
	(struct cp_genericize_omp_taskreg): New type.
	(struct cp_genericize_data): Add omp_ctx field.
	(cp_genericize_r): Attempt to determine implicitly determined
	firstprivate class type variables.
	(cp_genericize): Clear omp_ctx.
	* Make-lang.in (cp/cp-gimplify.o): Depend on $(SPLAY_TREE_H).

	* testsuite/libgomp.c++/pr48869.C: New test.


	Jakub
Jason Merrill - May 18, 2011, 9:01 p.m.
On 05/11/2011 08:26 AM, Jakub Jelinek wrote:
> This patch duplicates parts of the gimplifier's work during genericization,

That seems unfortunate, but I'll accept your judgment that it's the 
least-bad solution.  The patch is OK.

Jason

Patch

--- gcc/cp/method.c.jj	2011-05-04 10:13:56.000000000 +0200
+++ gcc/cp/method.c	2011-05-10 18:47:31.000000000 +0200
@@ -843,10 +843,10 @@  locate_fn_flags (tree type, tree name, t
 /* Locate the dtor of TYPE.  */
 
 tree
-get_dtor (tree type)
+get_dtor (tree type, tsubst_flags_t complain)
 {
   tree fn = locate_fn_flags (type, complete_dtor_identifier, NULL_TREE,
-			     LOOKUP_NORMAL, tf_warning_or_error);
+			     LOOKUP_NORMAL, complain);
   if (fn == error_mark_node)
     return NULL_TREE;
   return fn;
@@ -883,13 +883,13 @@  get_default_ctor (tree type)
 /* Locate the copy ctor of TYPE.  */
 
 tree
-get_copy_ctor (tree type)
+get_copy_ctor (tree type, tsubst_flags_t complain)
 {
   int quals = (TYPE_HAS_CONST_COPY_CTOR (type)
 	       ? TYPE_QUAL_CONST : TYPE_UNQUALIFIED);
   tree argtype = build_stub_type (type, quals, false);
   tree fn = locate_fn_flags (type, complete_ctor_identifier, argtype,
-			     LOOKUP_NORMAL, tf_warning_or_error);
+			     LOOKUP_NORMAL, complain);
   if (fn == error_mark_node)
     return NULL_TREE;
   return fn;
--- gcc/cp/cp-tree.h.jj	2011-05-04 10:13:56.000000000 +0200
+++ gcc/cp/cp-tree.h	2011-05-10 18:47:31.000000000 +0200
@@ -5020,10 +5020,10 @@  extern tree lazily_declare_fn			(special
 extern tree skip_artificial_parms_for		(const_tree, tree);
 extern int num_artificial_parms_for		(const_tree);
 extern tree make_alias_for			(tree, tree);
-extern tree get_copy_ctor			(tree);
+extern tree get_copy_ctor			(tree, tsubst_flags_t);
 extern tree get_copy_assign			(tree);
 extern tree get_default_ctor			(tree);
-extern tree get_dtor				(tree);
+extern tree get_dtor				(tree, tsubst_flags_t);
 extern tree locate_ctor				(tree);
 
 /* In optimize.c */
--- gcc/cp/semantics.c.jj	2011-05-02 18:39:12.000000000 +0200
+++ gcc/cp/semantics.c	2011-05-10 18:47:31.000000000 +0200
@@ -3757,7 +3757,7 @@  cxx_omp_create_clause_info (tree c, tree
       if (need_default_ctor)
 	t = get_default_ctor (type);
       else
-	t = get_copy_ctor (type);
+	t = get_copy_ctor (type, tf_warning_or_error);
 
       if (t && !trivial_fn_p (t))
 	TREE_VEC_ELT (info, 0) = t;
@@ -3765,7 +3765,7 @@  cxx_omp_create_clause_info (tree c, tree
 
   if ((need_default_ctor || need_copy_ctor)
       && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
-    TREE_VEC_ELT (info, 1) = get_dtor (type);
+    TREE_VEC_ELT (info, 1) = get_dtor (type, tf_warning_or_error);
 
   if (need_copy_assignment)
     {
--- gcc/cp/cp-gimplify.c.jj	2011-05-04 10:13:56.000000000 +0200
+++ gcc/cp/cp-gimplify.c	2011-05-11 09:40:21.000000000 +0200
@@ -32,6 +32,7 @@  along with GCC; see the file COPYING3.  
 #include "hashtab.h"
 #include "pointer-set.h"
 #include "flags.h"
+#include "splay-tree.h"
 
 /* Local declarations.  */
 
@@ -731,10 +732,105 @@  cxx_int_tree_map_hash (const void *item)
   return ((const struct cxx_int_tree_map *)item)->uid;
 }
 
+/* A stable comparison routine for use with splay trees and DECLs.  */
+
+static int
+splay_tree_compare_decl_uid (splay_tree_key xa, splay_tree_key xb)
+{
+  tree a = (tree) xa;
+  tree b = (tree) xb;
+
+  return DECL_UID (a) - DECL_UID (b);
+}
+
+/* OpenMP context during genericization.  */
+
+struct cp_genericize_omp_taskreg
+{
+  bool is_parallel;
+  bool default_shared;
+  struct cp_genericize_omp_taskreg *outer;
+  splay_tree variables;
+};
+
+/* Return true if genericization should try to determine if
+   DECL is firstprivate or shared within task regions.  */
+
+static bool
+omp_var_to_track (tree decl)
+{
+  tree type = TREE_TYPE (decl);
+  if (is_invisiref_parm (decl))
+    type = TREE_TYPE (type);
+  while (TREE_CODE (type) == ARRAY_TYPE)
+    type = TREE_TYPE (type);
+  if (type == error_mark_node || !CLASS_TYPE_P (type))
+    return false;
+  if (TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL_P (decl))
+    return false;
+  if (cxx_omp_predetermined_sharing (decl) != OMP_CLAUSE_DEFAULT_UNSPECIFIED)
+    return false;
+  return true;
+}
+
+/* Note DECL use in OpenMP region OMP_CTX during genericization.  */
+
+static void
+omp_cxx_notice_variable (struct cp_genericize_omp_taskreg *omp_ctx, tree decl)
+{
+  splay_tree_node n = splay_tree_lookup (omp_ctx->variables,
+					 (splay_tree_key) decl);
+  if (n == NULL)
+    {
+      int flags = OMP_CLAUSE_DEFAULT_SHARED;
+      if (omp_ctx->outer)
+	omp_cxx_notice_variable (omp_ctx->outer, decl);
+      if (!omp_ctx->default_shared)
+	{
+	  struct cp_genericize_omp_taskreg *octx;
+
+	  for (octx = omp_ctx->outer; octx; octx = octx->outer)
+	    {
+	      n = splay_tree_lookup (octx->variables, (splay_tree_key) decl);
+	      if (n && n->value != OMP_CLAUSE_DEFAULT_SHARED)
+		{
+		  flags = OMP_CLAUSE_DEFAULT_FIRSTPRIVATE;
+		  break;
+		}
+	      if (octx->is_parallel)
+		break;
+	    }
+	  if (octx == NULL
+	      && (TREE_CODE (decl) == PARM_DECL
+		  || (!(TREE_STATIC (decl) || DECL_EXTERNAL (decl))
+		      && DECL_CONTEXT (decl) == current_function_decl)))
+	    flags = OMP_CLAUSE_DEFAULT_FIRSTPRIVATE;
+	  if (flags == OMP_CLAUSE_DEFAULT_FIRSTPRIVATE)
+	    {
+	      /* DECL is implicitly determined firstprivate in
+		 the current task construct.  Ensure copy ctor and
+		 dtor are instantiated, because during gimplification
+		 it will be already too late.  */
+	      tree type = TREE_TYPE (decl);
+	      if (is_invisiref_parm (decl))
+		type = TREE_TYPE (type);
+	      while (TREE_CODE (type) == ARRAY_TYPE)
+		type = TREE_TYPE (type);
+	      get_copy_ctor (type, tf_none);
+	      get_dtor (type, tf_none);
+	    }
+	}
+      splay_tree_insert (omp_ctx->variables, (splay_tree_key) decl, flags);
+    }
+}
+
+/* Genericization context.  */
+
 struct cp_genericize_data
 {
   struct pointer_set_t *p_set;
   VEC (tree, heap) *bind_expr_stack;
+  struct cp_genericize_omp_taskreg *omp_ctx;
 };
 
 /* Perform any pre-gimplification lowering of C++ front end trees to
@@ -747,6 +843,14 @@  cp_genericize_r (tree *stmt_p, int *walk
   struct cp_genericize_data *wtd = (struct cp_genericize_data *) data;
   struct pointer_set_t *p_set = wtd->p_set;
 
+  /* If in an OpenMP context, note var uses.  */
+  if (__builtin_expect (wtd->omp_ctx != NULL, 0)
+      && (TREE_CODE (stmt) == VAR_DECL
+	  || TREE_CODE (stmt) == PARM_DECL
+	  || TREE_CODE (stmt) == RESULT_DECL)
+      && omp_var_to_track (stmt))
+    omp_cxx_notice_variable (wtd->omp_ctx, stmt);
+
   if (is_invisiref_parm (stmt)
       /* Don't dereference parms in a thunk, pass the references through. */
       && !(DECL_THUNK_P (current_function_decl)
@@ -786,6 +890,10 @@  cp_genericize_r (tree *stmt_p, int *walk
   if (TREE_CODE (stmt) == ADDR_EXPR
       && is_invisiref_parm (TREE_OPERAND (stmt, 0)))
     {
+      /* If in an OpenMP context, note var uses.  */
+      if (__builtin_expect (wtd->omp_ctx != NULL, 0)
+	  && omp_var_to_track (TREE_OPERAND (stmt, 0)))
+	omp_cxx_notice_variable (wtd->omp_ctx, TREE_OPERAND (stmt, 0));
       *stmt_p = convert (TREE_TYPE (stmt), TREE_OPERAND (stmt, 0));
       *walk_subtrees = 0;
     }
@@ -808,6 +916,22 @@  cp_genericize_r (tree *stmt_p, int *walk
 	  }
 	break;
       case OMP_CLAUSE_PRIVATE:
+	/* Don't dereference an invisiref in OpenMP clauses.  */
+	if (is_invisiref_parm (OMP_CLAUSE_DECL (stmt)))
+	  *walk_subtrees = 0;
+	else if (wtd->omp_ctx != NULL)
+	  {
+	    /* Private clause doesn't cause any references to the
+	       var in outer contexts, avoid calling
+	       omp_cxx_notice_variable for it.  */
+	    struct cp_genericize_omp_taskreg *old = wtd->omp_ctx;
+	    wtd->omp_ctx = NULL;
+	    cp_walk_tree (&OMP_CLAUSE_DECL (stmt), cp_genericize_r,
+			  data, NULL);
+	    wtd->omp_ctx = old;
+	    *walk_subtrees = 0;
+	  }
+	break;
       case OMP_CLAUSE_SHARED:
       case OMP_CLAUSE_FIRSTPRIVATE:
       case OMP_CLAUSE_COPYIN:
@@ -876,6 +1000,25 @@  cp_genericize_r (tree *stmt_p, int *walk
 
   else if (TREE_CODE (stmt) == BIND_EXPR)
     {
+      if (__builtin_expect (wtd->omp_ctx != NULL, 0))
+	{
+	  tree decl;
+	  for (decl = BIND_EXPR_VARS (stmt); decl; decl = DECL_CHAIN (decl))
+	    if (TREE_CODE (decl) == VAR_DECL
+		&& !DECL_EXTERNAL (decl)
+		&& omp_var_to_track (decl))
+	      {
+		splay_tree_node n
+		  = splay_tree_lookup (wtd->omp_ctx->variables,
+				       (splay_tree_key) decl);
+		if (n == NULL)
+		  splay_tree_insert (wtd->omp_ctx->variables,
+				     (splay_tree_key) decl,
+				     TREE_STATIC (decl)
+				     ? OMP_CLAUSE_DEFAULT_SHARED
+				     : OMP_CLAUSE_DEFAULT_PRIVATE);
+	      }
+	}
       VEC_safe_push (tree, heap, wtd->bind_expr_stack, stmt);
       cp_walk_tree (&BIND_EXPR_BODY (stmt),
 		    cp_genericize_r, data, NULL);
@@ -922,6 +1065,50 @@  cp_genericize_r (tree *stmt_p, int *walk
       *stmt_p = build1 (NOP_EXPR, void_type_node, integer_zero_node);
       *walk_subtrees = 0;
     }
+  else if (TREE_CODE (stmt) == OMP_PARALLEL || TREE_CODE (stmt) == OMP_TASK)
+    {
+      struct cp_genericize_omp_taskreg omp_ctx;
+      tree c, decl;
+      splay_tree_node n;
+
+      *walk_subtrees = 0;
+      cp_walk_tree (&OMP_CLAUSES (stmt), cp_genericize_r, data, NULL);
+      omp_ctx.is_parallel = TREE_CODE (stmt) == OMP_PARALLEL;
+      omp_ctx.default_shared = omp_ctx.is_parallel;
+      omp_ctx.outer = wtd->omp_ctx;
+      omp_ctx.variables = splay_tree_new (splay_tree_compare_decl_uid, 0, 0);
+      wtd->omp_ctx = &omp_ctx;
+      for (c = OMP_CLAUSES (stmt); c; c = OMP_CLAUSE_CHAIN (c))
+	switch (OMP_CLAUSE_CODE (c))
+	  {
+	  case OMP_CLAUSE_SHARED:
+	  case OMP_CLAUSE_PRIVATE:
+	  case OMP_CLAUSE_FIRSTPRIVATE:
+	  case OMP_CLAUSE_LASTPRIVATE:
+	    decl = OMP_CLAUSE_DECL (c);
+	    if (decl == error_mark_node || !omp_var_to_track (decl))
+	      break;
+	    n = splay_tree_lookup (omp_ctx.variables, (splay_tree_key) decl);
+	    if (n != NULL)
+	      break;
+	    splay_tree_insert (omp_ctx.variables, (splay_tree_key) decl,
+			       OMP_CLAUSE_CODE (c) == OMP_CLAUSE_SHARED
+			       ? OMP_CLAUSE_DEFAULT_SHARED
+			       : OMP_CLAUSE_DEFAULT_PRIVATE);
+	    if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_PRIVATE
+		&& omp_ctx.outer)
+	      omp_cxx_notice_variable (omp_ctx.outer, decl);
+	    break;
+	  case OMP_CLAUSE_DEFAULT:
+	    if (OMP_CLAUSE_DEFAULT_KIND (c) == OMP_CLAUSE_DEFAULT_SHARED)
+	      omp_ctx.default_shared = true;
+	  default:
+	    break;
+	  }
+      cp_walk_tree (&OMP_BODY (stmt), cp_genericize_r, data, NULL);
+      wtd->omp_ctx = omp_ctx.outer;
+      splay_tree_delete (omp_ctx.variables);
+    }
 
   pointer_set_insert (p_set, *stmt_p);
 
@@ -985,6 +1172,7 @@  cp_genericize (tree fndecl)
      walk_tree's hash functionality.  */
   wtd.p_set = pointer_set_create ();
   wtd.bind_expr_stack = NULL;
+  wtd.omp_ctx = NULL;
   cp_walk_tree (&DECL_SAVED_TREE (fndecl), cp_genericize_r, &wtd, NULL);
   pointer_set_destroy (wtd.p_set);
   VEC_free (tree, heap, wtd.bind_expr_stack);
--- gcc/cp/Make-lang.in.jj	2011-05-04 10:13:56.000000000 +0200
+++ gcc/cp/Make-lang.in	2011-05-06 18:12:57.000000000 +0200
@@ -328,7 +328,7 @@  cp/parser.o: cp/parser.c $(CXX_TREE_H) $
   gt-cp-parser.h output.h $(TARGET_H) $(PLUGIN_H) intl.h \
   c-family/c-objc.h tree-pretty-print.h $(CXX_PARSER_H) $(TIMEVAR.H)
 cp/cp-gimplify.o: cp/cp-gimplify.c $(CXX_TREE_H) $(C_COMMON_H) \
-	$(TM_H) coretypes.h pointer-set.h tree-iterator.h
+	$(TM_H) coretypes.h pointer-set.h tree-iterator.h $(SPLAY_TREE_H)
 
 cp/name-lookup.o: cp/name-lookup.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
 	$(TM_H) $(CXX_TREE_H) $(TIMEVAR_H) gt-cp-name-lookup.h \
--- libgomp/testsuite/libgomp.c++/pr48869.C.jj	2011-05-11 09:44:03.000000000 +0200
+++ libgomp/testsuite/libgomp.c++/pr48869.C	2011-05-11 09:44:12.000000000 +0200
@@ -0,0 +1,68 @@ 
+// PR c++/48869
+// { dg-do run }
+// { dg-options "-std=gnu++0x" }
+
+template <const int N>
+struct A
+{
+  A () {}
+  A (const A&) = delete;
+  void foo () {}
+  ~A () {}
+};
+
+template <const int N>
+struct B
+{
+  B () {}
+  B (const B&) {}
+  void foo () {}
+  ~B () {}
+};
+
+void __attribute__((used))
+foo (B<6> b6)
+{
+  #pragma omp task
+    b6.foo ();
+}
+
+int
+main ()
+{
+  A<0> a0;
+  #pragma omp task shared(a0)
+    a0.foo ();
+  #pragma omp task default(shared)
+    a0.foo ();
+  #pragma omp parallel shared(a0)
+    #pragma omp task
+      a0.foo ();
+  #pragma omp task
+  {
+    A<1> a1;
+    a1.foo ();
+  }
+  B<0> b0;
+  #pragma omp task shared(b0)
+    b0.foo ();
+  B<1> b1;
+  #pragma omp task default(shared)
+    b1.foo ();
+  B<2> b2;
+  #pragma omp parallel shared(b2)
+    #pragma omp task
+      b2.foo ();
+  B<3> b3;
+  #pragma omp task
+    b3.foo ();
+  B<4> b4;
+  #pragma omp parallel private (b4)
+    #pragma omp task
+      b4.foo ();
+  B<5> b5;
+  #pragma omp parallel firstprivate (b5)
+    #pragma omp task
+      b5.foo ();
+  return 0;
+}