On Fri, Mar 07, 2025 at 11:49:37AM +0000, Richard Sandiford wrote:
> > + case TCTX_OMP_DEVICE_ADDR:
> > + if (!silent_p)
> > + error_at (loc, "SVE type %qT not allowed in target device clauses",
> > type);
>
> Is the final error message accurate? This TCTX value is used for clauses
> like use_device_addr, which AIUI are part of "target data". Maybe something
> like "SVE type %qT is not a valid device storage type"?
I think target device clauses is fine, sure, it applies to all of
target{, data, enter data, exit data, update} but I wouldn't read target as
target construct only.
In any case, none of the SVE types make sense in the
is_device_ptr/use_device_addr/use_device_ptr/has_device_addr clauses
unless we'd have offloading from aarch64 host to aarch64 offloading device
with the same properties, because those clauses assume something is on the
offloading device already or similar.
> But TBH I know little about omp, so I hope someone else can suggest
> something better.
>
> Otherwise the AArch64 bits look good to me, to the extent that I'm
> able to review them.
> > --- a/gcc/gimplify.cc
> > +++ b/gcc/gimplify.cc
> > @@ -9081,11 +9081,20 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx,
> > tree decl, bool in_code)
> > | GOVD_MAP_ALLOC_ONLY)) == flags)
> > {
> > tree type = TREE_TYPE (decl);
> > + location_t loc = DECL_SOURCE_LOCATION (decl);
> >
> > if (gimplify_omp_ctxp->target_firstprivatize_array_bases
> > && omp_privatize_by_reference (decl))
> > type = TREE_TYPE (type);
> > - if (!omp_mappable_type (type))
> > +
> > + if (!verify_type_context (loc, TCTX_OMP_MAP_IMP_REF, type))
> > + {
> > + /* Check if TYPE can appear in a target region.
> > + verify_type_context has already issued an error if it
> > + can't. */
> > + nflags |= GOVD_MAP | GOVD_EXPLICIT;
> > + }
Please avoid {} around single statement body.
Otherwise the middle-end changes look ok to me.
> > + else if (!omp_mappable_type (type))
> > {
> > error ("%qD referenced in target region does not have "
> > "a mappable type", decl);
> > @@ -12815,6 +12824,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> > *pre_p,
> > unsigned int flags;
> > tree decl;
> > auto_vec<omp_addr_token *, 10> addr_tokens;
> > + tree op = NULL_TREE;
> > + location_t loc = OMP_CLAUSE_LOCATION (c);
> >
> > if (grp_end && c == OMP_CLAUSE_CHAIN (grp_end))
> > {
> > @@ -12822,6 +12833,36 @@ gimplify_scan_omp_clauses (tree *list_p,
> > gimple_seq *pre_p,
> > grp_end = NULL_TREE;
> > }
> >
> > + if (code == OMP_TARGET
> > + || code == OMP_TARGET_DATA
> > + || code == OMP_TARGET_ENTER_DATA
> > + || code == OMP_TARGET_EXIT_DATA)
> > + /* Do some target-specific type checks for map operands. */
> > + switch (OMP_CLAUSE_CODE (c))
> > + {
> > + case OMP_CLAUSE_MAP:
> > + op = OMP_CLAUSE_OPERAND (c, 0);
> > + verify_type_context (loc, TCTX_OMP_MAP, TREE_TYPE (op));
> > + break;
> > + case OMP_CLAUSE_PRIVATE:
> > + op = OMP_CLAUSE_OPERAND (c, 0);
> > + verify_type_context (loc, TCTX_OMP_PRIVATE, TREE_TYPE (op));
> > + break;
> > + case OMP_CLAUSE_FIRSTPRIVATE:
> > + op = OMP_CLAUSE_OPERAND (c, 0);
> > + verify_type_context (loc, TCTX_OMP_FIRSTPRIVATE, TREE_TYPE (op));
> > + break;
> > + case OMP_CLAUSE_IS_DEVICE_PTR:
> > + case OMP_CLAUSE_USE_DEVICE_ADDR:
> > + case OMP_CLAUSE_USE_DEVICE_PTR:
> > + case OMP_CLAUSE_HAS_DEVICE_ADDR:
> > + op = OMP_CLAUSE_OPERAND (c, 0);
> > + verify_type_context (loc, TCTX_OMP_DEVICE_ADDR, TREE_TYPE (op));
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > switch (OMP_CLAUSE_CODE (c))
> > {
> > case OMP_CLAUSE_PRIVATE:
> > diff --git a/gcc/target.h b/gcc/target.h
> > index 3e1ee68a341..618c9c03614 100644
> > --- a/gcc/target.h
> > +++ b/gcc/target.h
> > @@ -274,7 +274,24 @@ enum type_context_kind {
> > TCTX_EXCEPTIONS,
> >
> > /* Capturing objects of type T by value in a closure. */
> > - TCTX_CAPTURE_BY_COPY
> > + TCTX_CAPTURE_BY_COPY,
> > +
> > + /* Objects of type T appearing in OpenMP map clause. */
> > + TCTX_OMP_MAP,
> > +
> > + /* Objects of type T appearing in OpenMP target region
> > + without explicit map. */
> > + TCTX_OMP_MAP_IMP_REF,
> > +
> > + /* Objects of type T appearing in OpenMP private clause. */
> > + TCTX_OMP_PRIVATE,
> > +
> > + /* Objects of type T appearing in OpenMP firstprivate clause. */
> > + TCTX_OMP_FIRSTPRIVATE,
> > +
> > + /* Objects of type T appearing in OpenMP device clauses. */
> > + TCTX_OMP_DEVICE_ADDR
> > +
> > };
> >
> > enum poly_value_estimate_kind
> > @@ -331,6 +348,24 @@ mode_can_transfer_bits (machine_mode mode)
> > return true;
> > }
> >
> > +/* Return true if OpenMP context types. */
> > +
> > +inline bool
> > +omp_type_context (type_context_kind context)
> > +{
> > + switch (context)
> > + {
> > + case TCTX_OMP_MAP:
> > + case TCTX_OMP_MAP_IMP_REF:
> > + case TCTX_OMP_PRIVATE:
> > + case TCTX_OMP_FIRSTPRIVATE:
> > + case TCTX_OMP_DEVICE_ADDR:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > #ifdef GCC_TM_H
> >
> > #ifndef CUMULATIVE_ARGS_MAGIC
Jakub
