On 04/07/2016 02:56 AM, Jakub Jelinek wrote: > But more importantly, do you really have to do this separately? > I admit I haven't stepped through your testcases in the debugger, so > correct me if I'm missing something: > I mean, gimplify_scan_omp_clauses should omp_add_variable for the > OMP_CLAUSE_REDUCTION with > GOVD_REDUCTION | GOVD_SEEN | GOVD_EXPLICIT > and OMP_CLAUSE_MAP with GOVD_MAP | GOVD_EXPLICIT or so, similarly > GOVD_PRIVATE and/or GOVD_FIRSTPRIVATE flags from > OMP_CLAUSE_PRIVATE/OMP_CLAUSE_FIRSTPRIVATE. > So I believe you should have all the info you need in > (gimplify_adjust_omp_clauses) <case OMP_CLAUSE_REDUCTION>, > you have the CODE of the construct this is on (so check OACC_PARALLEL > or whatever you need), and you should be able to check if there is > explicit map/private/firstprivate clause together with OMP_CLAUSE_REDUCTION > or not, and then you can add the extra implicit clause and set > OMP_CLAUSE_MAP_IN_REDUCTION on it as appropriate.
This new patch does that now. Is this patch OK for trunk? Cesar
2016-04-07 Cesar Philippidis <ce...@codesourcery.com> PR lto/70289 PR ipa/70348 PR tree-optimization/70373 PR middle-end/70533 PR middle-end/70534 PR middle-end/70535 gcc/ * gimplify.c (omp_add_variable): Ignore seen reductions in ORT_ACC_PARALLEL regions. (gimplify_scan_omp_clauses): Adjust data clauses for acc parallel reductions as necessary. (gimplify_adjust_omp_clauses): Likewise, but add instead of adjust. * omp-low.c (is_oacc_parallel_reduction): New function. (scan_sharing_clauses): Use it to prevent installing local variables for those used in acc parallel reductions. (lower_rec_input_clauses): Remove dead code. (lower_oacc_reductions): Add support for reference reductions. (lower_reduction_clauses): Remove dead code. (lower_omp_target): Don't remap variables appearing in acc parallel reductions. * tree.h (OMP_CLAUSE_MAP_IN_REDUCTION): New macro. gcc/testsuite/ * c-c++-common/goacc/reduction-5.c: New test. * c-c++-common/goacc/reduction-promotions.c: New test. * gfortran.dg/goacc/reduction-3.f95: New test. * gfortran.dg/goacc/reduction-promotions.f90: New test. libgomp/ * testsuite/libgomp.oacc-c-c++-common/loop-reduction-gang-np-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-gw-np-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-gwv-np-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-gwv-np-2.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-gwv-np-3.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-gwv-np-4.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-vector-p-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-vector-p-2.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-worker-p-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-wv-p-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-wv-p-2.c: New test. * testsuite/libgomp.oacc-c-c++-common/loop-reduction-wv-p-3.c: New test. * testsuite/libgomp.oacc-c-c++-common/par-loop-comb-reduction-1.c: New test. * testsuite/libgomp.oacc-c-c++-common/par-loop-comb-reduction-2.c: New test. * testsuite/libgomp.oacc-c-c++-common/par-loop-comb-reduction-3.c: New test. * testsuite/libgomp.oacc-c-c++-common/par-loop-comb-reduction-4.c: New test. * testsuite/libgomp.oacc-c-c++-common/par-reduction-1.c: Add test coverage. * testsuite/libgomp.oacc-c-c++-common/par-reduction-2.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/parallel-dims.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/parallel-reduction.c: New test. * testsuite/libgomp.oacc-c-c++-common/pr70289.c: New test. * testsuite/libgomp.oacc-c-c++-common/pr70373.c: New test. * testsuite/libgomp.oacc-c-c++-common/reduction-1.c: Add test coverage. * testsuite/libgomp.oacc-c-c++-common/reduction-2.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/reduction-3.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/reduction-4.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/reduction-5.c: Likewise. * testsuite/libgomp.oacc-c-c++-common/reduction-6.c: New test. * testsuite/libgomp.oacc-c-c++-common/reduction.h: New test. * testsuite/libgomp.oacc-fortran/parallel-reduction.f90: New test. * testsuite/libgomp.oacc-fortran/pr70289.f90: New test. * testsuite/libgomp.oacc-fortran/reduction-1.f90: Add test coverage. * testsuite/libgomp.oacc-fortran/reduction-2.f90: Likewise. * testsuite/libgomp.oacc-fortran/reduction-3.f90: Likewise. * testsuite/libgomp.oacc-fortran/reduction-4.f90: Likewise. * testsuite/libgomp.oacc-fortran/reduction-5.f90: Likewise. * testsuite/libgomp.oacc-fortran/reduction-6.f90: Likewise. * testsuite/libgomp.oacc-fortran/reduction-7.f90: New test. diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 9c0119e..f53f5f3 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -5802,7 +5802,8 @@ omp_add_variable (struct gimplify_omp_ctx *ctx, tree decl, unsigned int flags) flags |= GOVD_SEEN; n = splay_tree_lookup (ctx->variables, (splay_tree_key)decl); - if (n != NULL && (n->value & GOVD_DATA_SHARE_CLASS) != 0) + if (n != NULL && (n->value & GOVD_DATA_SHARE_CLASS) != 0 + && ctx->region_type != ORT_ACC_PARALLEL) { /* We shouldn't be re-adding the decl with the same data sharing class. */ @@ -6557,6 +6558,24 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, break; } +/* OpenACC parallel reductions need a present_or_copy clause to ensure + that the original variable used in the reduction gets updated on + the host. Scan the list of clauses for reduction so that any existing + data clause can be adjusted if necessary. */ + if (region_type == ORT_ACC_PARALLEL) + { + for (c = *list_p; c; c = OMP_CLAUSE_CHAIN (c)) + { + tree decl = NULL_TREE; + + if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_REDUCTION) + continue; + + decl = OMP_CLAUSE_DECL (c); + omp_add_variable (ctx, decl, GOVD_REDUCTION); + } + } + while ((c = *list_p) != NULL) { bool remove = false; @@ -6808,6 +6827,31 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, case OMP_CLAUSE_MAP: decl = OMP_CLAUSE_DECL (c); + if (region_type == ORT_ACC_PARALLEL) + { + tree t = DECL_P (decl) ? decl : TREE_OPERAND (decl, 0); + splay_tree_node n = NULL; + + if (DECL_P (t)) + n = splay_tree_lookup (ctx->variables, (splay_tree_key)t); + + if (n) + { + int kind = OMP_CLAUSE_MAP_KIND (c); + + OMP_CLAUSE_MAP_IN_REDUCTION(c) = 1; + if ((kind & GOMP_MAP_TOFROM) != GOMP_MAP_TOFROM + && kind != GOMP_MAP_FORCE_PRESENT + && kind != GOMP_MAP_POINTER) + { + warning_at (OMP_CLAUSE_LOCATION (c), 0, + "incompatible data clause with reduction " + "on %qE; promoting to present_or_copy", + DECL_NAME (t)); + OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_TOFROM); + } + } + } if (error_operand_p (decl)) remove = true; switch (code) @@ -7269,6 +7313,16 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p, omp_notice_variable (outer_ctx, t, true); } } + /* Error on private reductions in OpenACC parallel directives. */ + if (region_type == ORT_ACC_PARALLEL + && (flags & (GOVD_PRIVATE | GOVD_FIRSTPRIVATE))) + { + splay_tree_node n = splay_tree_lookup (ctx->variables, + (splay_tree_key)decl); + if (n != NULL && (n->value & GOVD_REDUCTION)) + error_at (OMP_CLAUSE_LOCATION (c), "invalid private reduction " + "on %qE", DECL_NAME (decl)); + } omp_add_variable (ctx, decl, flags); if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION && OMP_CLAUSE_REDUCTION_PLACEHOLDER (c)) @@ -8118,6 +8172,29 @@ gimplify_adjust_omp_clauses (gimple_seq *pre_p, gimple_seq body, tree *list_p, case OMP_CLAUSE_REDUCTION: decl = OMP_CLAUSE_DECL (c); + /* OpenACC reductions need a present_or_copy data clause. + Add one if necessary. */ + if (ctx->region_type == ORT_ACC_PARALLEL) + { + n = splay_tree_lookup (ctx->variables, (splay_tree_key)decl); + if ((n->value & (GOVD_MAP | GOVD_PRIVATE | GOVD_FIRSTPRIVATE)) + == 0) + { + tree next = OMP_CLAUSE_CHAIN (c); + tree nc = build_omp_clause (UNKNOWN_LOCATION, OMP_CLAUSE_MAP); + OMP_CLAUSE_SET_MAP_KIND (nc, GOMP_MAP_TOFROM); + OMP_CLAUSE_DECL (nc) = decl; + OMP_CLAUSE_CHAIN (c) = nc; + lang_hooks.decls.omp_finish_clause (nc, pre_p); + for (; nc; nc = OMP_CLAUSE_CHAIN (nc)) + { + OMP_CLAUSE_MAP_IN_REDUCTION (nc) = 1; + if (OMP_CLAUSE_CHAIN (nc) == NULL) + break; + } + OMP_CLAUSE_CHAIN (nc) = next; + } + } if (DECL_P (decl) && omp_shared_to_firstprivate_optimizable_decl_p (decl)) omp_mark_stores (gimplify_omp_ctxp->outer_context, decl); diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 979926d..ed47853 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -2122,7 +2122,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx, else install_var_field (decl, true, 3, ctx, base_pointers_restrict); - if (is_gimple_omp_offloaded (ctx->stmt)) + if (is_gimple_omp_offloaded (ctx->stmt) + && !OMP_CLAUSE_MAP_IN_REDUCTION (c)) install_var_local (decl, ctx); } } @@ -4839,7 +4840,7 @@ lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist, gimplify_assign (ptr, x, ilist); } } - else if (is_reference (var) && !is_oacc_parallel (ctx)) + else if (is_reference (var)) { /* For references that are being privatized for Fortran, allocate new backing storage for the new pointer @@ -5575,7 +5576,8 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner, tree orig = OMP_CLAUSE_DECL (c); tree var = maybe_lookup_decl (orig, ctx); tree ref_to_res = NULL_TREE; - tree incoming, outgoing; + tree incoming, outgoing, v1, v2, v3; + bool is_private = false; enum tree_code rcode = OMP_CLAUSE_REDUCTION_CODE (c); if (rcode == MINUS_EXPR) @@ -5588,7 +5590,6 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner, if (!var) var = orig; - gcc_assert (!is_reference (var)); incoming = outgoing = var; @@ -5624,22 +5625,38 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner, for (; cls; cls = OMP_CLAUSE_CHAIN (cls)) if (OMP_CLAUSE_CODE (cls) == OMP_CLAUSE_REDUCTION && orig == OMP_CLAUSE_DECL (cls)) - goto has_outer_reduction; + { + incoming = outgoing = lookup_decl (orig, probe); + goto has_outer_reduction; + } + else if ((OMP_CLAUSE_CODE (cls) == OMP_CLAUSE_FIRSTPRIVATE + || OMP_CLAUSE_CODE (cls) == OMP_CLAUSE_PRIVATE) + && orig == OMP_CLAUSE_DECL (cls)) + { + is_private = true; + goto do_lookup; + } } do_lookup: /* This is the outermost construct with this reduction, see if there's a mapping for it. */ if (gimple_code (outer->stmt) == GIMPLE_OMP_TARGET - && maybe_lookup_field (orig, outer)) + && maybe_lookup_field (orig, outer) && !is_private) { ref_to_res = build_receiver_ref (orig, false, outer); if (is_reference (orig)) ref_to_res = build_simple_mem_ref (ref_to_res); + tree type = TREE_TYPE (var); + if (POINTER_TYPE_P (type)) + type = TREE_TYPE (type); + outgoing = var; - incoming = omp_reduction_init_op (loc, rcode, TREE_TYPE (var)); + incoming = omp_reduction_init_op (loc, rcode, type); } + else if (ctx->outer) + incoming = outgoing = lookup_decl (orig, ctx->outer); else incoming = outgoing = orig; @@ -5649,6 +5666,37 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner, if (!ref_to_res) ref_to_res = integer_zero_node; + if (is_reference (orig)) + { + tree type = TREE_TYPE (var); + const char *id = IDENTIFIER_POINTER (DECL_NAME (var)); + + if (!inner) + { + tree x = create_tmp_var (TREE_TYPE (type), id); + gimplify_assign (var, build_fold_addr_expr (x), fork_seq); + } + + v1 = create_tmp_var (type, id); + v2 = create_tmp_var (type, id); + v3 = create_tmp_var (type, id); + + gimplify_assign (v1, var, fork_seq); + gimplify_assign (v2, var, fork_seq); + gimplify_assign (v3, var, fork_seq); + + var = build_simple_mem_ref (var); + v1 = build_simple_mem_ref (v1); + v2 = build_simple_mem_ref (v2); + v3 = build_simple_mem_ref (v3); + outgoing = build_simple_mem_ref (outgoing); + + if (TREE_CODE (incoming) != INTEGER_CST) + incoming = build_simple_mem_ref (incoming); + } + else + v1 = v2 = v3 = var; + /* Determine position in reduction buffer, which may be used by target. */ enum machine_mode mode = TYPE_MODE (TREE_TYPE (var)); @@ -5678,20 +5726,20 @@ lower_oacc_reductions (location_t loc, tree clauses, tree level, bool inner, = build_call_expr_internal_loc (loc, IFN_GOACC_REDUCTION, TREE_TYPE (var), 6, init_code, unshare_expr (ref_to_res), - var, level, op, off); + v1, level, op, off); tree fini_call = build_call_expr_internal_loc (loc, IFN_GOACC_REDUCTION, TREE_TYPE (var), 6, fini_code, unshare_expr (ref_to_res), - var, level, op, off); + v2, level, op, off); tree teardown_call = build_call_expr_internal_loc (loc, IFN_GOACC_REDUCTION, TREE_TYPE (var), 6, teardown_code, - ref_to_res, var, level, op, off); + ref_to_res, v3, level, op, off); - gimplify_assign (var, setup_call, &before_fork); - gimplify_assign (var, init_call, &after_fork); - gimplify_assign (var, fini_call, &before_join); + gimplify_assign (v1, setup_call, &before_fork); + gimplify_assign (v2, init_call, &after_fork); + gimplify_assign (v3, fini_call, &before_join); gimplify_assign (outgoing, teardown_call, &after_join); } @@ -5933,9 +5981,6 @@ lower_reduction_clauses (tree clauses, gimple_seq *stmt_seqp, omp_context *ctx) } } - if (is_gimple_omp_oacc (ctx->stmt)) - return; - stmt = gimple_build_call (builtin_decl_explicit (BUILT_IN_GOMP_ATOMIC_START), 0); gimple_seq_add_stmt (stmt_seqp, stmt); @@ -15829,7 +15874,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) if (!maybe_lookup_field (var, ctx)) continue; - if (offloaded) + /* Don't remap oacc parallel reduction variables, because the + intermediate result must be local to each gang. */ + if (offloaded && !(OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP + && OMP_CLAUSE_MAP_IN_REDUCTION(c))) { x = build_receiver_ref (var, true, ctx); tree new_var = lookup_decl (var, ctx); diff --git a/gcc/tree.h b/gcc/tree.h index fa70596..87e7563 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -1536,6 +1536,9 @@ extern void protected_set_expr_location (tree, location_t); treatment if OMP_CLAUSE_SIZE is zero. */ #define OMP_CLAUSE_MAP_MAYBE_ZERO_LENGTH_ARRAY_SECTION(NODE) \ TREE_PROTECTED (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_MAP)) +/* Nonzero if this map clause is for an ACC parallel reduction variable. */ +#define OMP_CLAUSE_MAP_IN_REDUCTION(NODE) \ + TREE_PRIVATE (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_MAP)) #define OMP_CLAUSE_PROC_BIND_KIND(NODE) \ (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_PROC_BIND)->omp_clause.subcode.proc_bind_kind)
pr70533-20160407-full.diff.gz
Description: application/gzip