From 8ca5b2a2d4d6e210374f933c4c09895912d5e4ac Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 2 May 2006 22:03:38 +0200 Subject: [PATCH] re PR c++/26943 ([gomp] firstprivate + lastprivate uses inefficient barrier) PR c++/26943 * omp-low.c (maybe_lookup_decl_in_outer_ctx): New function. (build_outer_var_ref): Use maybe_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 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. From-SVN: r113483 --- gcc/ChangeLog | 15 ++++++++ gcc/omp-low.c | 65 +++++++++++++++++++++++++++------ libgomp/ChangeLog | 10 +++++ libgomp/testsuite/libgomp.c++/pr26943.C | 62 +++++++++++++++++++++++++++++++ libgomp/testsuite/libgomp.c++/pr27337.C | 6 +-- libgomp/testsuite/libgomp.c/pr26943-1.c | 24 ++++++++++++ libgomp/testsuite/libgomp.c/pr26943-2.c | 47 ++++++++++++++++++++++++ libgomp/testsuite/libgomp.c/pr26943-3.c | 56 ++++++++++++++++++++++++++++ libgomp/testsuite/libgomp.c/pr26943-4.c | 61 +++++++++++++++++++++++++++++++ 9 files changed, 330 insertions(+), 16 deletions(-) create mode 100644 libgomp/testsuite/libgomp.c++/pr26943.C create mode 100644 libgomp/testsuite/libgomp.c/pr26943-1.c create mode 100644 libgomp/testsuite/libgomp.c/pr26943-2.c create mode 100644 libgomp/testsuite/libgomp.c/pr26943-3.c create mode 100644 libgomp/testsuite/libgomp.c/pr26943-4.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ad825fc..162af2f 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,18 @@ +2006-05-02 Jakub Jelinek + + PR c++/26943 + * omp-low.c (maybe_lookup_decl_in_outer_ctx): New function. + (build_outer_var_ref): Use maybe_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 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. + 2006-05-02 Zdenek Dvorak * tree.c (unsigned_type_for, signed_type_for): Make sure a type diff --git a/gcc/omp-low.c b/gcc/omp-low.c index f344e69..2de13ec 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -112,6 +112,8 @@ 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 *); +static tree maybe_lookup_decl_in_outer_ctx (tree, omp_context *); /* Find an OpenMP clause of type KIND within CLAUSES. */ @@ -560,7 +562,7 @@ build_outer_var_ref (tree var, omp_context *ctx) { tree x; - if (is_global_var (var)) + if (is_global_var (maybe_lookup_decl_in_outer_ctx (var, ctx))) x = var; else if (is_variable_sized (var)) { @@ -674,9 +676,6 @@ omp_copy_decl (tree var, copy_body_data *cb) 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 +694,9 @@ omp_copy_decl (tree var, copy_body_data *cb) return new_var; } + if (is_global_var (var) || decl_function_context (var) != ctx->cb.src_fn) + return var; + return error_mark_node; } @@ -937,6 +939,10 @@ scan_sharing_clauses (tree clauses, omp_context *ctx) 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. */ + if (is_global_var (maybe_lookup_decl_in_outer_ctx (decl, ctx))) + break; if (! TREE_READONLY (decl) || TREE_ADDRESSABLE (decl) || by_ref @@ -963,7 +969,9 @@ scan_sharing_clauses (tree clauses, omp_context *ctx) do_private: if (is_variable_sized (decl)) break; - else if (is_parallel_ctx (ctx)) + else if (is_parallel_ctx (ctx) + && ! is_global_var (maybe_lookup_decl_in_outer_ctx (decl, + ctx))) { by_ref = use_pointer_for_field (decl, false); install_var_field (decl, by_ref, ctx); @@ -1029,7 +1037,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx) case OMP_CLAUSE_SHARED: decl = OMP_CLAUSE_DECL (c); - fixup_remapped_decl (decl, ctx, false); + if (! is_global_var (maybe_lookup_decl_in_outer_ctx (decl, ctx))) + fixup_remapped_decl (decl, ctx, false); break; case OMP_CLAUSE_COPYPRIVATE: @@ -1415,6 +1424,23 @@ lookup_decl_in_outer_ctx (tree decl, omp_context *ctx) } +/* Similar to lookup_decl_in_outer_ctx, but return DECL if not found + in outer contexts. */ + +static tree +maybe_lookup_decl_in_outer_ctx (tree decl, omp_context *ctx) +{ + tree t = NULL; + omp_context *up; + + if (ctx->is_nested) + for (up = ctx->outer, t = NULL; up && t == NULL; up = up->outer) + t = maybe_lookup_decl (decl, up); + + return t ? t : decl; +} + + /* Construct the initialization value for reduction CLAUSE. */ tree @@ -1493,6 +1519,7 @@ lower_rec_input_clauses (tree clauses, tree *ilist, tree *dlist, 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 +1545,22 @@ lower_rec_input_clauses (tree clauses, tree *ilist, tree *dlist, 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 +1646,9 @@ lower_rec_input_clauses (tree clauses, tree *ilist, tree *dlist, 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 +1738,10 @@ lower_rec_input_clauses (tree clauses, tree *ilist, tree *dlist, /* 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 +1959,9 @@ lower_send_clauses (tree clauses, tree *ilist, tree *olist, omp_context *ctx) 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); diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog index 754d0f9..76d5631 100644 --- a/libgomp/ChangeLog +++ b/libgomp/ChangeLog @@ -1,5 +1,15 @@ 2006-05-02 Jakub Jelinek + PR c++/26943 + * 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. + +2006-05-02 Jakub Jelinek + PR middle-end/27337 * testsuite/libgomp.c++/pr27337.C: New test. diff --git a/libgomp/testsuite/libgomp.c++/pr26943.C b/libgomp/testsuite/libgomp.c++/pr26943.C new file mode 100644 index 0000000..07b7b5d --- /dev/null +++ b/libgomp/testsuite/libgomp.c++/pr26943.C @@ -0,0 +1,62 @@ +// PR c++/26943 +// { dg-do run } + +#include +#include + +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; +} diff --git a/libgomp/testsuite/libgomp.c++/pr27337.C b/libgomp/testsuite/libgomp.c++/pr27337.C index c12154e..6db2465 100644 --- a/libgomp/testsuite/libgomp.c++/pr27337.C +++ b/libgomp/testsuite/libgomp.c++/pr27337.C @@ -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; } diff --git a/libgomp/testsuite/libgomp.c/pr26943-1.c b/libgomp/testsuite/libgomp.c/pr26943-1.c new file mode 100644 index 0000000..86c43f0 --- /dev/null +++ b/libgomp/testsuite/libgomp.c/pr26943-1.c @@ -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; +} diff --git a/libgomp/testsuite/libgomp.c/pr26943-2.c b/libgomp/testsuite/libgomp.c/pr26943-2.c new file mode 100644 index 0000000..7780484 --- /dev/null +++ b/libgomp/testsuite/libgomp.c/pr26943-2.c @@ -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; +} diff --git a/libgomp/testsuite/libgomp.c/pr26943-3.c b/libgomp/testsuite/libgomp.c/pr26943-3.c new file mode 100644 index 0000000..be93cb4 --- /dev/null +++ b/libgomp/testsuite/libgomp.c/pr26943-3.c @@ -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; +} diff --git a/libgomp/testsuite/libgomp.c/pr26943-4.c b/libgomp/testsuite/libgomp.c/pr26943-4.c new file mode 100644 index 0000000..33d3685 --- /dev/null +++ b/libgomp/testsuite/libgomp.c/pr26943-4.c @@ -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; +} -- 2.7.4