Minor follow-up comments:
Paul-Antoine Arras wrote:
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.
...
@@ -4067,23 +4069,125 @@ gimplify_call_expr
...
+ // Mark mapped argument as device pointer to ensure
+ // idempotency in gimplification
+ gcc_assert (gimplify_omp_ctxp->code == OMP_DISPATCH);
Just an observation: This assert looks as if it really should never
become true if one reads the code flow. I think it is therefore a
candidate for: gcc_checking_assert
While gcc_assert is enabled by --enable-checking=assert_checking, which
is on by default, gcc_checking_assert is enabled by …=checking, which is
enabled by default only when gcc/DEV-PHASE == experimental.
Thus, in releases this saves a few bytes (code + string) and µs
execution time ...
* * *
+/* Gimplify an OMP_DISPATCH construct. */
+
+static enum gimplify_status
+gimplify_omp_dispatch (tree *expr_p, gimple_seq *pre_p)
+{
...
+ // If device clause, adjust ICV
+ tree device
+ = omp_find_clause (OMP_DISPATCH_CLAUSES (expr), OMP_CLAUSE_DEVICE);
+ tree saved_device_icv;
+ if (device)
+ {
I think you should do:
if (device
&& (TREE_CODE (TREE_VALUE (device)) != INTEGER_CST
|| !wi::eq_p (device, -1 /* omp_initial_device */ )))
--- a/gcc/omp-builtins.def
+++ b/gcc/omp-builtins.def
@@ -80,6 +80,12 @@ DEF_GOMP_BUILTIN (BUILT_IN_OMP_GET_TEAM_NUM,
"omp_get_team_num",
BT_FN_INT, ATTR_CONST_NOTHROW_LEAF_LIST)
DEF_GOMP_BUILTIN (BUILT_IN_OMP_GET_NUM_TEAMS, "omp_get_num_teams",
BT_FN_INT, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GOMP_BUILTIN (BUILT_IN_OMP_GET_MAPPED_PTR, "omp_get_mapped_ptr",
+ BT_FN_PTR_CONST_PTR_INT, ATTR_NOTHROW_LEAF_LIST)
That's 'void *omp_get_mapped_ptr (const void *, int)'.
When using the function (the builtin), the pointer address of the first
argument escapes for the compiler, but we know it doesn't. Thus, I think
we want to use 'fn attr' here
→ builtin_fnspec in builtins.cc and attr-fnspec.h for the syntax.
Thanks,
Tobias
