This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[gomp] Fix a couple of global var handling bugs, add firstprivate+lastprivate barrier (PR c++/26943)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Henderson <rth at redhat dot com>, Diego Novillo <dnovillo at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 1 May 2006 13:06:11 -0400
- Subject: [gomp] Fix a couple of global var handling bugs, add firstprivate+lastprivate barrier (PR c++/26943)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
As the testcases show, there were several bugs in handling is_global_var
variables - the sender and receiver side did not agree on where they were
passed etc.
In addition to that, we need a barrier after firstprivate initialization
if there are any vars in both firstprivate and lastprivate clauses.
Eventually, we might avoid that barrier in some specific cases, see
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26943#c5
but correctness first.
Ok for trunk?
2006-05-01 Jakub Jelinek <jakub@redhat.com>
PR c++/26943
* omp-low.c (build_outer_var_ref): Use lookup_decl_in_outer_ctx
to find if var will be a global variable even in the nested context.
(omp_copy_decl): Only check for global variable at the end, it might
be overridden in outer contexts.
(scan_sharing_clauses): For global variables in non-nested contexts
don't create a field.
(lower_rec_input_clauses): Do nothing for global shared variables.
Emit a barrier at the end of ILIST if there were any decls in both
firstprivate and lastprivate clauses.
(lower_send_clauses): Do nothing for global variables except for
COPYIN.
* testsuite/libgomp.c/pr26943-1.c: New test.
* testsuite/libgomp.c/pr26943-2.c: New test.
* testsuite/libgomp.c/pr26943-3.c: New test.
* testsuite/libgomp.c/pr26943-4.c: New test.
* testsuite/libgomp.c++/pr27337.C: Remove barrier.
* testsuite/libgomp.c++/pr26943.C: New test.
--- gcc/omp-low.c.jj 2006-04-28 13:29:49.000000000 +0200
+++ gcc/omp-low.c 2006-05-01 18:54:29.000000000 +0200
@@ -112,6 +112,7 @@ struct omp_region *root_omp_region;
static void scan_omp (tree *, omp_context *);
static void lower_omp (tree *, omp_context *);
+static tree lookup_decl_in_outer_ctx (tree, omp_context *);
/* Find an OpenMP clause of type KIND within CLAUSES. */
@@ -558,9 +559,11 @@ build_receiver_ref (tree var, bool by_re
static tree
build_outer_var_ref (tree var, omp_context *ctx)
{
- tree x;
+ tree x, new_var = var;
- if (is_global_var (var))
+ if (ctx->is_nested && is_global_var (var))
+ new_var = lookup_decl_in_outer_ctx (var, ctx);
+ if (is_global_var (new_var))
x = var;
else if (is_variable_sized (var))
{
@@ -674,9 +677,6 @@ omp_copy_decl (tree var, copy_body_data
omp_context *ctx = (omp_context *) cb;
tree new_var;
- if (is_global_var (var) || decl_function_context (var) != ctx->cb.src_fn)
- return var;
-
if (TREE_CODE (var) == LABEL_DECL)
{
new_var = create_artificial_label ();
@@ -695,6 +695,9 @@ omp_copy_decl (tree var, copy_body_data
return new_var;
}
+ if (is_global_var (var) || decl_function_context (var) != ctx->cb.src_fn)
+ return var;
+
return error_mark_node;
}
@@ -937,6 +940,12 @@ scan_sharing_clauses (tree clauses, omp_
decl = OMP_CLAUSE_DECL (c);
gcc_assert (!is_variable_sized (decl));
by_ref = use_pointer_for_field (decl, true);
+ /* Global variables don't need to be copied,
+ the receiver side will use them directly.
+ For nested contexts we unfortunately don't know
+ yet whether it will be a global variable or not. */
+ if (! ctx->is_nested && is_global_var (decl))
+ break;
if (! TREE_READONLY (decl)
|| TREE_ADDRESSABLE (decl)
|| by_ref
@@ -963,7 +972,8 @@ scan_sharing_clauses (tree clauses, omp_
do_private:
if (is_variable_sized (decl))
break;
- else if (is_parallel_ctx (ctx))
+ else if (is_parallel_ctx (ctx)
+ && (ctx->is_nested || ! is_global_var (decl)))
{
by_ref = use_pointer_for_field (decl, false);
install_var_field (decl, by_ref, ctx);
@@ -1029,7 +1039,8 @@ scan_sharing_clauses (tree clauses, omp_
case OMP_CLAUSE_SHARED:
decl = OMP_CLAUSE_DECL (c);
- fixup_remapped_decl (decl, ctx, false);
+ if (ctx->is_nested || ! is_global_var (decl))
+ fixup_remapped_decl (decl, ctx, false);
break;
case OMP_CLAUSE_COPYPRIVATE:
@@ -1493,6 +1504,7 @@ lower_rec_input_clauses (tree clauses, t
tree_stmt_iterator diter;
tree c, dtor, copyin_seq, x, args, ptr;
bool copyin_by_ref = false;
+ bool lastprivate_firstprivate = false;
int pass;
*dlist = alloc_stmt_list ();
@@ -1518,14 +1530,22 @@ lower_rec_input_clauses (tree clauses, t
continue;
break;
case OMP_CLAUSE_SHARED:
+ if (maybe_lookup_decl (OMP_CLAUSE_DECL (c), ctx) == NULL)
+ {
+ gcc_assert (is_global_var (OMP_CLAUSE_DECL (c)));
+ continue;
+ }
case OMP_CLAUSE_FIRSTPRIVATE:
case OMP_CLAUSE_COPYIN:
case OMP_CLAUSE_REDUCTION:
break;
case OMP_CLAUSE_LASTPRIVATE:
- if (pass != 0
- && OMP_CLAUSE_LASTPRIVATE_FIRSTPRIVATE (c))
- continue;
+ if (OMP_CLAUSE_LASTPRIVATE_FIRSTPRIVATE (c))
+ {
+ lastprivate_firstprivate = true;
+ if (pass != 0)
+ continue;
+ }
break;
default:
continue;
@@ -1611,6 +1631,9 @@ lower_rec_input_clauses (tree clauses, t
switch (OMP_CLAUSE_CODE (c))
{
case OMP_CLAUSE_SHARED:
+ /* Shared global vars are just accessed directly. */
+ if (is_global_var (new_var))
+ break;
/* 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. */
@@ -1700,8 +1723,10 @@ lower_rec_input_clauses (tree clauses, t
/* If any copyin variable is passed by reference, we must ensure the
master thread doesn't modify it before it is copied over in all
- threads. */
- if (copyin_by_ref)
+ threads. Similarly for variables in both firstprivate and
+ lastprivate clauses we need to ensure the lastprivate copying
+ happens after firstprivate copying in all threads. */
+ if (copyin_by_ref || lastprivate_firstprivate)
build_omp_barrier (ilist);
}
@@ -1919,6 +1944,9 @@ lower_send_clauses (tree clauses, tree *
if (ctx->is_nested)
var = lookup_decl_in_outer_ctx (val, ctx);
+ if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_COPYIN
+ && is_global_var (var))
+ continue;
if (is_variable_sized (val))
continue;
by_ref = use_pointer_for_field (val, false);
--- libgomp/testsuite/libgomp.c/pr26943-1.c.jj 2006-05-01 15:50:07.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr26943-1.c 2006-05-01 15:49:48.000000000 +0200
@@ -0,0 +1,24 @@
+/* PR c++/26943 */
+/* { dg-do run } */
+
+extern void abort (void);
+extern void omp_set_dynamic (int);
+int n = 6;
+
+int
+main (void)
+{
+ int i, x = 0;
+ omp_set_dynamic (0);
+#pragma omp parallel for num_threads (16) firstprivate (n) lastprivate (n) \
+ schedule (static, 1) reduction (+: x)
+ for (i = 0; i < 16; i++)
+ {
+ if (n != 6)
+ ++x;
+ n = i;
+ }
+ if (x || n != 15)
+ abort ();
+ return 0;
+}
--- libgomp/testsuite/libgomp.c/pr26943-2.c.jj 2006-05-01 16:41:34.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr26943-2.c 2006-05-01 16:42:01.000000000 +0200
@@ -0,0 +1,47 @@
+/* PR c++/26943 */
+/* { dg-do run } */
+
+extern int omp_set_dynamic (int);
+extern void abort (void);
+
+int a = 8, b = 12, c = 16, d = 20, j = 0;
+char e[10] = "a", f[10] = "b", g[10] = "c", h[10] = "d";
+
+int
+main (void)
+{
+ int i;
+ omp_set_dynamic (0);
+#pragma omp parallel for shared (a, e) firstprivate (b, f) \
+ lastprivate (c, g) private (d, h) \
+ schedule (static, 1) num_threads (4) \
+ reduction (+:j)
+ for (i = 0; i < 4; i++)
+ {
+ if (a != 8 || b != 12 || e[0] != 'a' || f[0] != 'b')
+ j++;
+#pragma omp barrier
+#pragma omp atomic
+ a += i;
+ b += i;
+ c = i;
+ d = i;
+#pragma omp atomic
+ e[0] += i;
+ f[0] += i;
+ g[0] = 'g' + i;
+ h[0] = 'h' + i;
+#pragma omp barrier
+ if (a != 8 + 6 || b != 12 + i || c != i || d != i)
+ j += 8;
+ if (e[0] != 'a' + 6 || f[0] != 'b' + i || g[0] != 'g' + i)
+ j += 64;
+ if (h[0] != 'h' + i)
+ j += 512;
+ }
+ if (j || a != 8 + 6 || b != 12 || c != 3 || d != 20)
+ abort ();
+ if (e[0] != 'a' + 6 || f[0] != 'b' || g[0] != 'g' + 3 || h[0] != 'd')
+ abort ();
+ return 0;
+}
--- libgomp/testsuite/libgomp.c/pr26943-3.c.jj 2006-05-01 16:41:34.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr26943-3.c 2006-05-01 17:56:57.000000000 +0200
@@ -0,0 +1,56 @@
+/* PR c++/26943 */
+/* { dg-do run } */
+
+extern int omp_set_dynamic (int);
+extern int omp_get_thread_num (void);
+extern void abort (void);
+
+int a = 8, b = 12, c = 16, d = 20, j = 0, l = 0;
+char e[10] = "a", f[10] = "b", g[10] = "c", h[10] = "d";
+volatile int k;
+
+int
+main (void)
+{
+ int i;
+ omp_set_dynamic (0);
+ omp_set_nested (1);
+#pragma omp parallel num_threads (2) reduction (+:l)
+ if (k == omp_get_thread_num ())
+ {
+#pragma omp parallel for shared (a, e) firstprivate (b, f) \
+ lastprivate (c, g) private (d, h) \
+ schedule (static, 1) num_threads (4) \
+ reduction (+:j)
+ for (i = 0; i < 4; i++)
+ {
+ if (a != 8 || b != 12 || e[0] != 'a' || f[0] != 'b')
+ j++;
+#pragma omp barrier
+#pragma omp atomic
+ a += i;
+ b += i;
+ c = i;
+ d = i;
+#pragma omp atomic
+ e[0] += i;
+ f[0] += i;
+ g[0] = 'g' + i;
+ h[0] = 'h' + i;
+#pragma omp barrier
+ if (a != 8 + 6 || b != 12 + i || c != i || d != i)
+ j += 8;
+ if (e[0] != 'a' + 6 || f[0] != 'b' + i || g[0] != 'g' + i)
+ j += 64;
+ if (h[0] != 'h' + i)
+ j += 512;
+ }
+ if (j || a != 8 + 6 || b != 12 || c != 3 || d != 20)
+ ++l;
+ if (e[0] != 'a' + 6 || f[0] != 'b' || g[0] != 'g' + 3 || h[0] != 'd')
+ l += 8;
+ }
+ if (l)
+ abort ();
+ return 0;
+}
--- libgomp/testsuite/libgomp.c/pr26943-4.c.jj 2006-05-01 16:41:34.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr26943-4.c 2006-05-01 18:36:08.000000000 +0200
@@ -0,0 +1,61 @@
+/* PR c++/26943 */
+/* { dg-do run } */
+
+extern int omp_set_dynamic (int);
+extern int omp_get_thread_num (void);
+extern void abort (void);
+
+int a = 8, b = 12, c = 16, d = 20, j = 0, l = 0;
+char e[10] = "a", f[10] = "b", g[10] = "c", h[10] = "d";
+volatile int k;
+
+int
+main (void)
+{
+ int i;
+ omp_set_dynamic (0);
+ omp_set_nested (1);
+#pragma omp parallel num_threads (2) reduction (+:l) \
+ firstprivate (a, b, c, d, e, f, g, h, j)
+ if (k == omp_get_thread_num ())
+ {
+#pragma omp parallel for shared (a, e) firstprivate (b, f) \
+ lastprivate (c, g) private (d, h) \
+ schedule (static, 1) num_threads (4) \
+ reduction (+:j)
+ for (i = 0; i < 4; i++)
+ {
+ if (a != 8 || b != 12 || e[0] != 'a' || f[0] != 'b')
+ j++;
+#pragma omp barrier
+#pragma omp atomic
+ a += i;
+ b += i;
+ c = i;
+ d = i;
+#pragma omp atomic
+ e[0] += i;
+ f[0] += i;
+ g[0] = 'g' + i;
+ h[0] = 'h' + i;
+#pragma omp barrier
+ if (a != 8 + 6 || b != 12 + i || c != i || d != i)
+ j += 8;
+ if (e[0] != 'a' + 6 || f[0] != 'b' + i || g[0] != 'g' + i)
+ j += 64;
+ if (h[0] != 'h' + i)
+ j += 512;
+ }
+ if (j || a != 8 + 6 || b != 12 || c != 3 || d != 20)
+ ++l;
+ if (e[0] != 'a' + 6 || f[0] != 'b' || g[0] != 'g' + 3 || h[0] != 'd')
+ l += 8;
+ }
+ if (l)
+ abort ();
+ if (a != 8 || b != 12 || c != 16 || d != 20)
+ abort ();
+ if (e[0] != 'a' || f[0] != 'b' || g[0] != 'c' || h[0] != 'd')
+ abort ();
+ return 0;
+}
--- libgomp/testsuite/libgomp.c++/pr27337.C.jj 2006-04-27 18:30:16.000000000 +0200
+++ libgomp/testsuite/libgomp.c++/pr27337.C 2006-05-01 16:22:36.000000000 +0200
@@ -48,11 +48,7 @@ foo ()
#pragma omp parallel for firstprivate (ret) lastprivate (ret) \
schedule (static, 1) num_threads (4)
for (i = 0; i < 4; i++)
- {
- ret.i += omp_get_thread_num ();
- /* FIXME: The following barrier should be unnecessary. */
-#pragma omp barrier
- }
+ ret.i += omp_get_thread_num ();
return ret;
}
--- libgomp/testsuite/libgomp.c++/pr26943.C.jj 2006-05-01 15:42:17.000000000 +0200
+++ libgomp/testsuite/libgomp.c++/pr26943.C 2006-05-01 15:42:10.000000000 +0200
@@ -0,0 +1,62 @@
+// PR c++/26943
+// { dg-do run }
+
+#include <assert.h>
+#include <unistd.h>
+
+struct S
+{
+ public:
+ int x;
+ S () : x(-1) { }
+ S (const S &);
+ S& operator= (const S &);
+ void test ();
+};
+
+static volatile int hold;
+
+S::S (const S &s)
+{
+ #pragma omp master
+ sleep (1);
+
+ assert (s.x == -1);
+ x = 0;
+}
+
+S&
+S::operator= (const S& s)
+{
+ assert (s.x == 1);
+ x = 2;
+ return *this;
+}
+
+void
+S::test ()
+{
+ assert (x == 0);
+ x = 1;
+}
+
+static S x;
+
+void
+foo ()
+{
+ #pragma omp sections firstprivate(x) lastprivate(x)
+ {
+ x.test();
+ }
+}
+
+int
+main ()
+{
+ #pragma omp parallel num_threads(2)
+ foo();
+
+ assert (x.x == 2);
+ return 0;
+}
Jakub