On Wed, Feb 23, 2022 at 05:01:45PM +0100, Marcel Vollweiler wrote:
> gcc/cp/ChangeLog:
>
> * pt.cc (tsubst_omp_clauses): Add OMP_CLAUSE_HAS_DEVICE_ADDR.
> * semantics.cc (finish_omp_clauses): Handle PARM_DECL and
> NON_LVALUE_EXPR.
>
> gcc/ChangeLog:
>
> * gimplify.cc (gimplify_scan_omp_clauses): Handle NON_LVALUE_EXPR.
> (gimplify_adjust_omp_clauses): Likewise.
> * omp-low.cc (scan_sharing_clauses): Likewise.
> (lower_omp_target): Likewise.
>
> libgomp/ChangeLog:
>
> * testsuite/libgomp.c++/target-has-device-addr-7.C: New test.
> * testsuite/libgomp.c++/target-has-device-addr-8.C: New test.
> * testsuite/libgomp.c++/target-has-device-addr-9.C: New test.
>
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -17652,6 +17652,7 @@ tsubst_omp_clauses (tree clauses, enum
> c_omp_region_type ort,
> case OMP_CLAUSE_USE_DEVICE_PTR:
> case OMP_CLAUSE_USE_DEVICE_ADDR:
> case OMP_CLAUSE_IS_DEVICE_PTR:
> + case OMP_CLAUSE_HAS_DEVICE_ADDR:
> case OMP_CLAUSE_INCLUSIVE:
> case OMP_CLAUSE_EXCLUSIVE:
> OMP_CLAUSE_DECL (nc)
> @@ -17797,6 +17798,7 @@ tsubst_omp_clauses (tree clauses, enum
> c_omp_region_type ort,
> case OMP_CLAUSE_USE_DEVICE_PTR:
> case OMP_CLAUSE_USE_DEVICE_ADDR:
> case OMP_CLAUSE_IS_DEVICE_PTR:
> + case OMP_CLAUSE_HAS_DEVICE_ADDR:
> case OMP_CLAUSE_INCLUSIVE:
> case OMP_CLAUSE_EXCLUSIVE:
> case OMP_CLAUSE_ALLOCATE:
This part is ok.
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 0cb17a6..452ecfd 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -8534,11 +8534,14 @@ finish_omp_clauses (tree clauses, enum
> c_omp_region_type ort)
> {
> if (handle_omp_array_sections (c, ort))
> remove = true;
> + else if (TREE_CODE (TREE_CHAIN (t)) == PARM_DECL)
> + t = TREE_CHAIN (t);
> else
> {
> t = OMP_CLAUSE_DECL (c);
> while (TREE_CODE (t) == INDIRECT_REF
> - || TREE_CODE (t) == ARRAY_REF)
> + || TREE_CODE (t) == ARRAY_REF
> + || TREE_CODE (t) == NON_LVALUE_EXPR)
> t = TREE_OPERAND (t, 0);
> }
> }
This is wrong.
When processing_template_decl, handle_omp_array_sections often punts, keeps
things as is because if something is dependent, we can't do much about it.
The else if (TREE_CODE (TREE_CHAIN (t)) == PARM_DECL) is obviously wrong,
there is really nothing specific about PARM_DECLs (just that you used
exactly that in the testcase), nor about array section with exactly one
dimension. What is done elsewhere is look through all TREE_LISTs to find
the base expression, and if that expression is a VAR_DECL/PARM_DECL, nice,
we can do further processing, if processing_template_decl and it is
something different, just defer and otherwise error out.
So I think you want:
--- gcc/cp/semantics.cc.jj 2022-05-05 11:56:16.160443828 +0200
+++ gcc/cp/semantics.cc 2022-05-05 15:52:39.651211448 +0200
@@ -8553,14 +8553,23 @@ finish_omp_clauses (tree clauses, enum c
else
{
t = OMP_CLAUSE_DECL (c);
+ if (TREE_CODE (t) == TREE_LIST)
+ {
+ while (TREE_CODE (t) == TREE_LIST)
+ t = TREE_CHAIN (t);
+ }
while (TREE_CODE (t) == INDIRECT_REF
|| TREE_CODE (t) == ARRAY_REF)
t = TREE_OPERAND (t, 0);
}
}
- bitmap_set_bit (&is_on_device_head, DECL_UID (t));
if (VAR_P (t) || TREE_CODE (t) == PARM_DECL)
- cxx_mark_addressable (t);
+ {
+ bitmap_set_bit (&is_on_device_head, DECL_UID (t));
+ if (!processing_template_decl
+ && !cxx_mark_addressable (t))
+ remove = true;
+ }
goto check_dup_generic_t;
case OMP_CLAUSE_USE_DEVICE_ADDR:
instead, as I said look through the TREE_LISTs, then only use DECL_UID
on actual VAR_DECLs/PARM_DECLs not random other expressions and
never call cxx_mark_addressable when processing_template_decl (and remove
clause if cxx_mark_addressable fails).
Note, check_dup_generic_t will do among other things:
if (!VAR_P (t) && TREE_CODE (t) != PARM_DECL
&& (!field_ok || TREE_CODE (t) != FIELD_DECL))
{
if (processing_template_decl && TREE_CODE (t) != OVERLOAD)
break;
... error ...
}
so with processing_template_decl it will just defer it for later,
but otherwise if t is something invalid it will diagnose it.
But one really shouldn't rely on t being VAR_DECL/PARM_DECL before
that checking is done...
With your pt.cc change and my semantics.cc change, all your new testcases
look fine.
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index f570daa..b1bb5be 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -10285,7 +10285,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> case OMP_CLAUSE_HAS_DEVICE_ADDR:
> decl = OMP_CLAUSE_DECL (c);
> while (TREE_CODE (decl) == INDIRECT_REF
> - || TREE_CODE (decl) == ARRAY_REF)
> + || TREE_CODE (decl) == ARRAY_REF
> + || TREE_CODE (decl) == NON_LVALUE_EXPR)
> decl = TREE_OPERAND (decl, 0);
> flags = GOVD_EXPLICIT;
> goto do_add_decl;
> @@ -11443,7 +11444,8 @@ gimplify_adjust_omp_clauses (gimple_seq *pre_p,
> gimple_seq body, tree *list_p,
> case OMP_CLAUSE_HAS_DEVICE_ADDR:
> decl = OMP_CLAUSE_DECL (c);
> while (TREE_CODE (decl) == INDIRECT_REF
> - || TREE_CODE (decl) == ARRAY_REF)
> + || TREE_CODE (decl) == ARRAY_REF
> + || TREE_CODE (decl) == NON_LVALUE_EXPR)
> decl = TREE_OPERAND (decl, 0);
> n = splay_tree_lookup (ctx->variables, (splay_tree_key) decl);
> remove = n == NULL || !(n->value & GOVD_SEEN);
> diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
> index 77176ef..30cc9b6 100644
> --- a/gcc/omp-low.cc
> +++ b/gcc/omp-low.cc
> @@ -1384,7 +1384,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> }
> else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> {
> - if (TREE_CODE (decl) == INDIRECT_REF)
> + if (TREE_CODE (decl) == INDIRECT_REF
> + || TREE_CODE (decl) == NON_LVALUE_EXPR)
> decl = TREE_OPERAND (decl, 0);
> install_var_field (decl, true, 3, ctx);
> }
> @@ -1747,7 +1748,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> {
> while (TREE_CODE (decl) == INDIRECT_REF
> - || TREE_CODE (decl) == ARRAY_REF)
> + || TREE_CODE (decl) == ARRAY_REF
> + || TREE_CODE (decl) == NON_LVALUE_EXPR)
> decl = TREE_OPERAND (decl, 0);
> }
>
> @@ -12847,7 +12849,8 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
> omp_context *ctx)
> if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> {
> while (TREE_CODE (var) == INDIRECT_REF
> - || TREE_CODE (var) == ARRAY_REF)
> + || TREE_CODE (var) == ARRAY_REF
> + || TREE_CODE (var) == NON_LVALUE_EXPR)
> var = TREE_OPERAND (var, 0);
> }
> map_cnt++;
> @@ -13337,7 +13340,8 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
> omp_context *ctx)
> if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> {
> while (TREE_CODE (ovar) == INDIRECT_REF
> - || TREE_CODE (ovar) == ARRAY_REF)
> + || TREE_CODE (ovar) == ARRAY_REF
> + || TREE_CODE (ovar) == NON_LVALUE_EXPR)
> ovar = TREE_OPERAND (ovar, 0);
> }
> var = lookup_decl_in_outer_ctx (ovar, ctx);
> @@ -13607,7 +13611,8 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
> omp_context *ctx)
> if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> {
> while (TREE_CODE (var) == INDIRECT_REF
> - || TREE_CODE (var) == ARRAY_REF)
> + || TREE_CODE (var) == ARRAY_REF
> + || TREE_CODE (var) == NON_LVALUE_EXPR)
> var = TREE_OPERAND (var, 0);
> }
> x = build_receiver_ref (var, false, ctx);
So all the gimplify.cc and omp-low.cc changes look suspicious.
NON_LVALUE_EXPR shouldn't show up there...
Jakub
