re PR middle-end/35549 (Invalid use of copy-in/out for shared vars in nested parallels)

	PR middle-end/35549
	* omp-low.c (maybe_lookup_decl): Constify first argument.
	(use_pointer_for_field): Change last argument from bool to
	omp_context *.  Disallow shared copy-in/out in nested
	parallel if decl is shared in outer parallel too.
	(build_outer_var_ref, scan_sharing_clauses,
	lower_rec_input_clauses, lower_copyprivate_clauses,
	lower_send_clauses, lower_send_shared_vars): Adjust callers.

	* testsuite/libgomp.c/pr35549.c: New test.

From-SVN: r133136

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 5075eaf7633030769e67a52038badd97fdb9b1b7..db105a96e9322bcab081a2d3bcb3729723e03f15 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2008-03-12  Jakub Jelinek  <jakub@redhat.com>
+
+       PR middle-end/35549
+       * omp-low.c (maybe_lookup_decl): Constify first argument.
+       (use_pointer_for_field): Change last argument from bool to
+       omp_context *.  Disallow shared copy-in/out in nested
+       parallel if decl is shared in outer parallel too.
+       (build_outer_var_ref, scan_sharing_clauses,
+       lower_rec_input_clauses, lower_copyprivate_clauses,
+       lower_send_clauses, lower_send_shared_vars): Adjust callers.
+
 2008-03-12  Victor Kaplansky  <victork@il.ibm.com>
            Ira Rosen  <irar@il.ibm.com>
 

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 2e1a1b8aa8df46199af8a93a3e0f2cfa9d57b5bd..47114b6941f31291519fd88e913c61795e4dd71a 100644 (file)
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -456,7 +456,7 @@ lookup_decl (tree var, omp_context *ctx)
 }
 
 static inline tree
-maybe_lookup_decl (tree var, omp_context *ctx)
+maybe_lookup_decl (const_tree var, omp_context *ctx)
 {
   tree *n;
   n = (tree *) pointer_map_contains (ctx->cb.decl_map, var);
@@ -479,18 +479,18 @@ maybe_lookup_field (tree var, omp_context *ctx)
   return n ? (tree) n->value : NULL_TREE;
 }
 
-/* Return true if DECL should be copied by pointer.  SHARED_P is true
-   if DECL is to be shared.  */
+/* Return true if DECL should be copied by pointer.  SHARED_CTX is
+   the parallel context if DECL is to be shared.  */
 
 static bool
-use_pointer_for_field (const_tree decl, bool shared_p)
+use_pointer_for_field (const_tree decl, omp_context *shared_ctx)
 {
   if (AGGREGATE_TYPE_P (TREE_TYPE (decl)))
     return true;
 
   /* We can only use copy-in/copy-out semantics for shared variables
      when we know the value is not accessible from an outer scope.  */
-  if (shared_p)
+  if (shared_ctx)
     {
       /* ??? Trivially accessible from anywhere.  But why would we even
         be passing an address in this case?  Should we simply assert
@@ -510,6 +510,34 @@ use_pointer_for_field (const_tree decl, bool shared_p)
         address taken.  */
       if (TREE_ADDRESSABLE (decl))
        return true;
+
+      /* Disallow copy-in/out in nested parallel if
+        decl is shared in outer parallel, otherwise
+        each thread could store the shared variable
+        in its own copy-in location, making the
+        variable no longer really shared.  */
+      if (!TREE_READONLY (decl) && shared_ctx->is_nested)
+       {
+         omp_context *up;
+
+         for (up = shared_ctx->outer; up; up = up->outer)
+           if (maybe_lookup_decl (decl, up))
+             break;
+
+         if (up && is_parallel_ctx (up))
+           {
+             tree c;
+
+             for (c = OMP_PARALLEL_CLAUSES (up->stmt);
+                  c; c = OMP_CLAUSE_CHAIN (c))
+               if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_SHARED
+                   && OMP_CLAUSE_DECL (c) == decl)
+                 break;
+
+             if (c)
+               return true;
+           }
+       }
     }
 
   return false;
@@ -596,7 +624,7 @@ build_outer_var_ref (tree var, omp_context *ctx)
     }
   else if (is_parallel_ctx (ctx))
     {
-      bool by_ref = use_pointer_for_field (var, false);
+      bool by_ref = use_pointer_for_field (var, NULL);
       x = build_receiver_ref (var, by_ref, ctx);
     }
   else if (ctx->outer)
@@ -966,7 +994,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
          gcc_assert (is_parallel_ctx (ctx));
          decl = OMP_CLAUSE_DECL (c);
          gcc_assert (!is_variable_sized (decl));
-         by_ref = use_pointer_for_field (decl, true);
+         by_ref = use_pointer_for_field (decl, ctx);
          /* Global variables don't need to be copied,
             the receiver side will use them directly.  */
          if (is_global_var (maybe_lookup_decl_in_outer_ctx (decl, ctx)))
@@ -1001,7 +1029,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
                   && ! is_global_var (maybe_lookup_decl_in_outer_ctx (decl,
                                                                       ctx)))
            {
-             by_ref = use_pointer_for_field (decl, false);
+             by_ref = use_pointer_for_field (decl, NULL);
              install_var_field (decl, by_ref, ctx);
            }
          install_var_local (decl, ctx);
@@ -1014,7 +1042,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
 
        case OMP_CLAUSE_COPYIN:
          decl = OMP_CLAUSE_DECL (c);
-         by_ref = use_pointer_for_field (decl, false);
+         by_ref = use_pointer_for_field (decl, NULL);
          install_var_field (decl, by_ref, ctx);
          break;
 
@@ -1751,7 +1779,7 @@ lower_rec_input_clauses (tree clauses, tree *ilist, tree *dlist,
              /* Set up the DECL_VALUE_EXPR for shared variables now.  This
                 needs to be delayed until after fixup_child_record_type so
                 that we get the correct type during the dereference.  */
-             by_ref = use_pointer_for_field (var, true);
+             by_ref = use_pointer_for_field (var, ctx);
              x = build_receiver_ref (var, by_ref, ctx);
              SET_DECL_VALUE_EXPR (new_var, x);
              DECL_HAS_VALUE_EXPR_P (new_var) = 1;
@@ -1794,7 +1822,7 @@ lower_rec_input_clauses (tree clauses, tree *ilist, tree *dlist,
              break;
 
            case OMP_CLAUSE_COPYIN:
-             by_ref = use_pointer_for_field (var, false);
+             by_ref = use_pointer_for_field (var, NULL);
              x = build_receiver_ref (var, by_ref, ctx);
              x = lang_hooks.decls.omp_clause_assign_op (c, new_var, x);
              append_to_statement_list (x, &copyin_seq);
@@ -2007,7 +2035,7 @@ lower_copyprivate_clauses (tree clauses, tree *slist, tree *rlist,
        continue;
 
       var = OMP_CLAUSE_DECL (c);
-      by_ref = use_pointer_for_field (var, false);
+      by_ref = use_pointer_for_field (var, NULL);
 
       ref = build_sender_ref (var, ctx);
       x = lookup_decl_in_outer_ctx (var, ctx);
@@ -2059,7 +2087,7 @@ lower_send_clauses (tree clauses, tree *ilist, tree *olist, omp_context *ctx)
        continue;
       if (is_variable_sized (val))
        continue;
-      by_ref = use_pointer_for_field (val, false);
+      by_ref = use_pointer_for_field (val, NULL);
 
       switch (OMP_CLAUSE_CODE (c))
        {
@@ -2129,7 +2157,7 @@ lower_send_shared_vars (tree *ilist, tree *olist, omp_context *ctx)
         mapping for OVAR.  */
       var = lookup_decl_in_outer_ctx (ovar, ctx);
 
-      if (use_pointer_for_field (ovar, true))
+      if (use_pointer_for_field (ovar, ctx))
        {
          x = build_sender_ref (ovar, ctx);
          var = build_fold_addr_expr (var);

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 84c70491ece58d4d6137ef9280fa826af6ea470c..3078243448771033a434ac87f819b27cbba0b600 100644 (file)
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,8 @@
+2008-03-12  Jakub Jelinek  <jakub@redhat.com>
+
+       PR middle-end/35549
+       * testsuite/libgomp.c/pr35549.c: New test.
+
 2008-03-06  Jakub Jelinek  <jakub@redhat.com>
 
        * testsuite/libgomp.c/atomic-3.c: New test.

diff --git a/libgomp/testsuite/libgomp.c/pr35549.c b/libgomp/testsuite/libgomp.c/pr35549.c
new file mode 100644 (file)
index 0000000..269a0c2
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/pr35549.c
@@ -0,0 +1,30 @@
+/* PR middle-end/35549 */
+/* { dg-do run } */
+
+#include <omp.h>
+#include <stdlib.h>
+
+int
+main (void)
+{
+  int i = 6, n = 0;
+  omp_set_dynamic (0);
+  omp_set_nested (1);
+  #pragma omp parallel shared (i) num_threads (3)
+  {
+    if (omp_get_num_threads () != 3)
+      #pragma omp atomic
+       n += 1;
+    #pragma omp parallel shared (i) num_threads (4)
+    {
+      if (omp_get_num_threads () != 4)
+       #pragma omp atomic
+         n += 1;
+      #pragma omp critical
+       i += 1;
+    }
+  }
+  if (n == 0 && i != 6 + 3 * 4)
+    abort ();
+  return 0;
+}