Re: [PATCH v2 3/8] OpenMP: middle-end support for dispatch + adjust_args
Hi PA,
as discussed off list, I was stumbling over the call to GOMP_task. I now
understand why: I was looking at a different version of the OpenMP spec.
Namely, OpenMP 5.2 contains the changes for spec Issue 2741 "dispatch
construct data scoping issues". Namely: Performance issue due to 'task'
compared to direct call, effect of unintended firstprivatization, …
The currrent version has
(a) nowait
"The addition of the *nowait* element to the semantic requirement set by
the *dispatch* directive has no effect on the dispatch construct apart
from the effect it may have on the arguments that are passed when
calling a function variant." (I assume the latter is about 'append_args'
of interop objects)
(b) depend
"If the *dispatch* directive adds one or more _depend_ element to the
semantic requirement set, and those element are not removed by the
effect of a declare variant directive, the behavior is as if those
properties were applied as *depend* clauses to a *taskwait* construct
that is executed before the *dispatch* region is executed."
I think it would good to match the 5.2 behavior.
* * *
I have not fully checked whether the 'device' routine is properly
handled. The current wording states:
"If the device clause is present, the value of the default-device-var
ICV is set to the value of the expression in the clause on entry to the
dispatch region and is restored to its previous value at the end of the
region."
For the code itself, it seems to be handled correctly, see attached
testcase (consider including).
I was wondering (and haven't checked) whether the ICV is set for too
much (i.e. not only the "data environment" (i.e.
"The variables associated with the execution of a given region"), but is
also imminently visible by other concurrently running threads outside of
that region).
Can you check. (Albeit, my question might also be answered once I finish
reading the patch …)
Thanks,
Tobias
#include
int f ()
{
return omp_get_default_device ();
}
int main ()
{
for (int d = omp_initial_device; d <= omp_get_num_devices (); d++)
{
int dev = omp_invalid_device;
omp_set_default_device (d);
#pragma omp dispatch
dev = f ();
if (d == omp_initial_device || d == omp_get_num_devices ())
{
if (dev != omp_initial_device && dev != omp_get_num_devices ())
__builtin_abort ();
if (omp_get_default_device() != omp_initial_device
&& omp_get_default_device() != omp_get_num_devices ())
__builtin_abort ();
}
else
if (dev != d || d != omp_get_default_device())
__builtin_abort ();
for (int d2 = omp_initial_device; d2 <= omp_get_num_devices (); d2++)
{
dev = omp_invalid_device;
#pragma omp dispatch device(d2)
dev = f ();
if (d == omp_initial_device || d == omp_get_num_devices ())
{
if (omp_get_default_device() != omp_initial_device
&& omp_get_default_device() != omp_get_num_devices ())
__builtin_abort ();
}
else if (d != omp_get_default_device())
__builtin_abort ();
if (d2 == omp_initial_device || d2 == omp_get_num_devices ())
{
if (dev != omp_initial_device && dev != omp_get_num_devices ())
__builtin_abort ();
}
else if (dev != d2)
__builtin_abort ();
}
}
return 0;
}
Re: [PATCH v2 3/8] OpenMP: middle-end support for dispatch + adjust_args
Hi PA, not yet a full review, but some observations: First: Please include the change gcc/fortran/types.def (BT_FN_PTR_CONST_PTR_INT) of "[PATCH v2 7/8] OpenMP: Fortran front-end support for dispatch + adjust_args" Do so either in this patch (3/8) - or in the previous (2/8) one that adds it to gcc/builtin-types.def. Otherwise this will break the build as omp-builtins.def (modified in this patch) is also used by gfortran. Causing intermittened build fails is bad - first, in general, and secondly it causes issues when bisecting. * * * If I try your testcase and move "bar" and "baz" *after* 'foo' and leave only the following before: int baz (double *d_bv, const double *d_av, int n); int bar (double *d_bv, const double *d_av, int n); it fails at runtime with: ERROR at 1: 0.00 (act) != 2.718280 (exp) as the two calls to __builtin_omp_get_mapped_ptr are now missing. With both the declaration and the definition before the declare target, it works. * * * I think this variant needs to be either supported – or an error has to be printed that it cannot be supported, but that would be rather unfortunate. Thanks, Tobias
[PATCH v2 3/8] OpenMP: middle-end support for dispatch + adjust_args
This patch adds middle-end support for the `dispatch` construct and the
`adjust_args` clause. The heavy lifting is done in `gimplify_omp_dispatch` and
`gimplify_call_expr` respectively. For `adjust_args`, this mostly consists in
emitting a call to `gomp_get_mapped_ptr` for the adequate device.
For dispatch, the following steps are performed:
* Handle the device clause, if any. This may affect `need_device_ptr` arguments.
* Handle novariants and nocontext clauses, if any. Evaluate compile-time
constants and select a variant, if possible. Otherwise, emit code to handle all
possible cases at run time.
* Create an explicit task, as if the `task` construct was used, that wraps the
body of the `dispatch` statement. Move relevant clauses to the task.
gcc/ChangeLog:
* gimple-low.cc (lower_stmt): Handle GIMPLE_OMP_DISPATCH.
* gimple-pretty-print.cc (dump_gimple_omp_dispatch): New function.
(pp_gimple_stmt_1): Handle GIMPLE_OMP_DISPATCH.
* gimple-walk.cc (walk_gimple_stmt): Likewise.
* gimple.cc (gimple_build_omp_dispatch): New function.
(gimple_copy): Handle GIMPLE_OMP_DISPATCH.
* gimple.def (GIMPLE_OMP_DISPATCH): Define.
* gimple.h (gimple_build_omp_dispatch): Declare.
(gimple_has_substatements): Handle GIMPLE_OMP_DISPATCH.
(gimple_omp_dispatch_clauses): New function.
(gimple_omp_dispatch_clauses_ptr): Likewise.
(gimple_omp_dispatch_set_clauses): Likewise.
(gimple_return_set_retval): Handle GIMPLE_OMP_DISPATCH.
* gimplify.cc (enum omp_region_type): Add ORT_DISPATCH.
(gimplify_call_expr): Handle need_device_ptr arguments.
(is_gimple_stmt): Handle OMP_DISPATCH.
(gimplify_scan_omp_clauses): Handle OMP_CLAUSE_DEVICE in a dispatch
construct. Handle OMP_CLAUSE_NOVARIANTS and OMP_CLAUSE_NOCONTEXT.
(gimplify_adjust_omp_clauses): Handle OMP_CLAUSE_NOVARIANTS and
OMP_CLAUSE_NOCONTEXT.
(omp_construct_selector_matches): Handle OMP_DISPATCH with nocontext
clause.
(omp_has_novariants): New function.
(omp_has_nocontext): Likewise.
(gimplify_omp_dispatch): Likewise.
(gimplify_expr): Handle OMP_DISPATCH.
* gimplify.h (omp_has_novariants): Declare.
(omp_has_nocontext): Declare.
* omp-builtins.def (BUILT_IN_OMP_GET_MAPPED_PTR): Define.
(BUILT_IN_OMP_GET_DEFAULT_DEVICE): Define.
(BUILT_IN_OMP_SET_DEFAULT_DEVICE): Define.
* omp-expand.cc (expand_omp_dispatch): New function.
(expand_omp): Handle GIMPLE_OMP_DISPATCH.
(omp_make_gimple_edges): Likewise.
* omp-general.cc (omp_construct_traits_to_codes): Add OMP_DISPATCH.
(struct omp_ts_info): Add dispatch.
(omp_context_selector_matches): Handle OMP_TRAIT_SET_NEED_DEVICE_PTR.
(omp_resolve_declare_variant): Handle novariants. Adjust
DECL_ASSEMBLER_NAME.
---
gcc/gimple-low.cc | 1 +
gcc/gimple-pretty-print.cc | 33 +++
gcc/gimple-walk.cc | 1 +
gcc/gimple.cc | 20 ++
gcc/gimple.def | 5 +
gcc/gimple.h | 33 ++-
gcc/gimplify.cc| 412 -
gcc/gimplify.h | 2 +
gcc/omp-builtins.def | 6 +
gcc/omp-expand.cc | 18 ++
gcc/omp-general.cc | 16 +-
gcc/omp-low.cc | 35
gcc/tree-inline.cc | 7 +
13 files changed, 578 insertions(+), 11 deletions(-)
diff --git a/gcc/gimple-low.cc b/gcc/gimple-low.cc
index e0371988705..712a1ebf776 100644
--- a/gcc/gimple-low.cc
+++ b/gcc/gimple-low.cc
@@ -746,6 +746,7 @@ lower_stmt (gimple_stmt_iterator *gsi, struct lower_data
*data)
case GIMPLE_EH_MUST_NOT_THROW:
case GIMPLE_OMP_FOR:
case GIMPLE_OMP_SCOPE:
+case GIMPLE_OMP_DISPATCH:
case GIMPLE_OMP_SECTIONS:
case GIMPLE_OMP_SECTIONS_SWITCH:
case GIMPLE_OMP_SECTION:
diff --git a/gcc/gimple-pretty-print.cc b/gcc/gimple-pretty-print.cc
index 08b823c84ef..e7b2df9a0ef 100644
--- a/gcc/gimple-pretty-print.cc
+++ b/gcc/gimple-pretty-print.cc
@@ -1726,6 +1726,35 @@ dump_gimple_omp_scope (pretty_printer *pp, const gimple
*gs,
}
}
+/* Dump a GIMPLE_OMP_DISPATCH tuple on the pretty_printer BUFFER. */
+
+static void
+dump_gimple_omp_dispatch (pretty_printer *buffer, const gimple *gs, int spc,
+ dump_flags_t flags)
+{
+ if (flags & TDF_RAW)
+{
+ dump_gimple_fmt (buffer, spc, flags, "%G <%+BODY <%S>%nCLAUSES <", gs,
+ gimple_omp_body (gs));
+ dump_omp_clauses (buffer, gimple_omp_dispatch_clauses (gs), spc, flags);
+ dump_gimple_fmt (buffer, spc, flags, " >");
+}
+ else
+{
+ pp_string (buffer, "#pragma omp dispatch");
+ dump_omp_clauses (buffer, gimple_omp_dispatch_clauses (gs), spc, flags);
+ if (!gimple_seq_empty_p (gimple_omp_body (gs)))
+ {
+ newline_and_indent (buffer, spc + 2);
+ pp_lef
