Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
On Wed, Nov 04, 2020 at 02:02:25AM +0800, Chung-Lin Tang wrote: > gcc/c-family/ > * c-common.h (c_omp_adjust_map_clauses): New declaration. > * c-omp.c (c_omp_adjust_map_clauses): New function. > > gcc/c/ > * c-parser.c (c_parser_omp_target_data): Add use of > new c_omp_adjust_map_clauses function. Add GOMP_MAP_ATTACH_DETACH as > handled map clause kind. > (c_parser_omp_target_enter_data): Likewise. > (c_parser_omp_target_exit_data): Likewise. > (c_parser_omp_target): Likewise. > * c-typeck.c (handle_omp_array_sections): Adjust COMPONENT_REF case to > use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. > (c_finish_omp_clauses): Adjust bitmap checks to allow struct decl and > same struct field access to co-exist on OpenMP construct. > > gcc/cp/ > * parser.c (cp_parser_omp_target_data): Add use of > new c_omp_adjust_map_clauses function. Add GOMP_MAP_ATTACH_DETACH as > handled map clause kind. > (cp_parser_omp_target_enter_data): Likewise. > (cp_parser_omp_target_exit_data): Likewise. > (cp_parser_omp_target): Likewise. > * semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to > use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix > interaction between reference case and attach/detach. > (finish_omp_clauses): Adjust bitmap checks to allow struct decl and > same struct field access to co-exist on OpenMP construct. Ok, thanks. Jakub
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
Hi Jakub,
here is v3 of this patch set.
On 2020/10/29 7:44 PM, Jakub Jelinek wrote:
+extern void c_omp_adjust_clauses (tree, bool);
So, can you please rename the function to either
c_omp_adjust_target_clauses or c_omp_adjust_mapping_clauses or
c_omp_adjust_map_clauses?
I've renamed it to 'c_omp_adjust_map_clauses'.
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -2579,3 +2579,50 @@ c_omp_map_clause_name (tree clause, bool oacc)
}
return omp_clause_code_name[OMP_CLAUSE_CODE (clause)];
}
+
+/* Adjust map clauses after normal clause parsing, mainly to turn specific
+ base-pointer map cases into attach/detach and mark them addressable. */
+void
+c_omp_adjust_clauses (tree clauses, bool is_target)
+{
+ for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
+if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
If this is only meant to handle decls, perhaps there should be
&& DECL_P (OMP_CLAUSE_DECL (c))
?
+ && TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
+ {
+ tree ptr = OMP_CLAUSE_DECL (c);
+ bool ptr_mapped = false;
+ if (is_target)
+ {
+ for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
+ if (OMP_CLAUSE_CODE (m) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_DECL (m) == ptr
+ && (OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALLOC
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TO
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_FROM
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TOFROM
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_TO
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_FROM
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_TOFROM))
+ {
+ ptr_mapped = true;
+ break;
+ }
What you could e.g. do is have this loop at the start of function, with
&& DECL_P (OMP_CLAUSE_DECL (m))
instead of the == ptr check, and perhaps && POINTER_TYPE_P (TREE_TYPE
(OMP_CLAUSE_DECL (m))) check and set a bit in a bitmap for each such decl,
then in the GOMP_MAP_FIRSTPRIVATE_POINTER loop just check the bitmap.
Or, keep it in the loop like it is above, but populate the bitmap
lazily (upon seeing the first GOMP_MAP_FIRSTPRIVATE_POINTER) and for further
ones just use it.
I re-wrote c_omp_adjust_map_clauses to address the complexity issues you
mentioned,
now it should be limited by a linear pass to collect and merge the firstprivate
base
pointer + existence of a mapping of it, using a hash_map.
Patch set has been re-tested with no regressions for gcc, g++, gfortran, and
libgomp.
Thanks,
Chung-Lin
gcc/c-family/
* c-common.h (c_omp_adjust_map_clauses): New declaration.
* c-omp.c (c_omp_adjust_map_clauses): New function.
gcc/c/
* c-parser.c (c_parser_omp_target_data): Add use of
new c_omp_adjust_map_clauses function. Add GOMP_MAP_ATTACH_DETACH as
handled map clause kind.
(c_parser_omp_target_enter_data): Likewise.
(c_parser_omp_target_exit_data): Likewise.
(c_parser_omp_target): Likewise.
* c-typeck.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type.
(c_finish_omp_clauses): Adjust bitmap checks to allow struct decl and
same struct field access to co-exist on OpenMP construct.
gcc/cp/
* parser.c (cp_parser_omp_target_data): Add use of
new c_omp_adjust_map_clauses function. Add GOMP_MAP_ATTACH_DETACH as
handled map clause kind.
(cp_parser_omp_target_enter_data): Likewise.
(cp_parser_omp_target_exit_data): Likewise.
(cp_parser_omp_target): Likewise.
* semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix
interaction between reference case and attach/detach.
(finish_omp_clauses): Adjust bitmap checks to allow struct decl and
same struct field access to co-exist on OpenMP construct.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index bb38e6c76a4..3eb909a2946 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1221,6 +1221,7 @@ extern enum omp_clause_defaultmap_kind
c_omp_predetermined_mapping (tree);
extern tree c_omp_check_context_selector (location_t, tree);
extern void c_omp_mark_declare_variant (location_t, tree, tree);
extern const char *c_omp_map_clause_name (tree, bool);
+extern void c_omp_adjust_map_clauses (tree, bool);
/* Return next tree in the chain for chain_next walking of tree nodes. */
static inline tree
diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index d7cff0f4cca..275c6afabe1 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -2579,3 +2579,92 @@
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
On Wed, Oct 28, 2020 at 06:31:22PM +0800, Chung-Lin Tang wrote:
> > > + for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> > > +if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> > > + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
> > > + && TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
> > > + {
> > > + tree ptr = OMP_CLAUSE_DECL (c);
> > > + bool ptr_mapped = false;
> > > + if (is_target)
> > > + {
> > > + for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
> > Isn't this O(n^2) in number of clauses? I mean, e.g. for the equality
> > comparisons (but see below) it could be dealt with e.g. using some bitmap
> > with DECL_UIDs.
>
> At this stage, we really don't assume any ordering of the clauses, nor try to
> modify its ordering yet, so the base-pointer map (if it exists) could be any
> where in the list (building some "visited set" isn't really suitable here).
> I don't think this is really that much an issue of concern though.
Many functions try hard to avoid O(n^2) issues, see e.g. all the bitmap
handling in *finish_omp_clauses etc.
One can have tens of thousands of clauses and then the quadraticness will
hit hard. This does a mere OMP_CLAUSE_DECL (c) == ptr comparison, so it
is only about the decls and decls can be very easily handled through
DECL_UID (bitmaps, hash sets/maps/tables).
> +extern void c_omp_adjust_clauses (tree, bool);
So, can you please rename the function to either
c_omp_adjust_target_clauses or c_omp_adjust_mapping_clauses or
c_omp_adjust_map_clauses?
> --- a/gcc/c-family/c-omp.c
> +++ b/gcc/c-family/c-omp.c
> @@ -2579,3 +2579,50 @@ c_omp_map_clause_name (tree clause, bool oacc)
> }
>return omp_clause_code_name[OMP_CLAUSE_CODE (clause)];
> }
> +
> +/* Adjust map clauses after normal clause parsing, mainly to turn specific
> + base-pointer map cases into attach/detach and mark them addressable. */
> +void
> +c_omp_adjust_clauses (tree clauses, bool is_target)
> +{
> + for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> +if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
If this is only meant to handle decls, perhaps there should be
&& DECL_P (OMP_CLAUSE_DECL (c))
?
> + && TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
> + {
> + tree ptr = OMP_CLAUSE_DECL (c);
> + bool ptr_mapped = false;
> + if (is_target)
> + {
> + for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
> + if (OMP_CLAUSE_CODE (m) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_DECL (m) == ptr
> + && (OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALLOC
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TO
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_FROM
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TOFROM
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_TO
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_FROM
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALWAYS_TOFROM))
> + {
> + ptr_mapped = true;
> + break;
> + }
What you could e.g. do is have this loop at the start of function, with
&& DECL_P (OMP_CLAUSE_DECL (m))
instead of the == ptr check, and perhaps && POINTER_TYPE_P (TREE_TYPE
(OMP_CLAUSE_DECL (m))) check and set a bit in a bitmap for each such decl,
then in the GOMP_MAP_FIRSTPRIVATE_POINTER loop just check the bitmap.
Or, keep it in the loop like it is above, but populate the bitmap
lazily (upon seeing the first GOMP_MAP_FIRSTPRIVATE_POINTER) and for further
ones just use it.
Jakub
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
Hi Jakub,
Thank you for the review.
On 2020/10/13 9:01 PM, Jakub Jelinek wrote:
gcc/c-family/
* c-common.h (c_omp_adjust_clauses): New declaration.
* c-omp.c (c_omp_adjust_clauses): New function.
Besides the naming, I wonder why is it done in a separate function and so
early, can't what the function does be done either in
{,c_}finish_omp_clauses (provided we'd pass separate ORT_OMP vs.
ORT_OMP_TARGET to it to determine if it is target region vs. anything else),
or perhaps even better during gimplification (gimplify_scan_omp_clauses)?
I figured that differentiating with something like "C_ORT_OMP_TARGET" could be
more error prone to adjust changes related to C_ORT_OMP across the code, plus
this has the added benefit of sharing a single place of handling logic across
C/C++.
You're right about the need for early addressable-marking. Learned that the hard
way, one of my prior attempts tried to place this code somewhere in gimplify,
didn't work.
gcc/cp/
* parser.c (cp_parser_omp_target_data): Add use of
new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as
handled map clause kind.
(cp_parser_omp_target_enter_data): Likewise.
(cp_parser_omp_target_exit_data): Likewise.
(cp_parser_omp_target): Likewise.
* semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix
interaction between reference case and attach/detach.
(finish_omp_clauses): Adjust bitmap checks to allow struct decl and
same struct field access to co-exist on OpenMP construct.
The changelog has some 8 space indented lines.
I'll take care of that in the final git push.
+ for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
+if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
+ && TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
+ {
+ tree ptr = OMP_CLAUSE_DECL (c);
+ bool ptr_mapped = false;
+ if (is_target)
+ {
+ for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
Isn't this O(n^2) in number of clauses? I mean, e.g. for the equality
comparisons (but see below) it could be dealt with e.g. using some bitmap
with DECL_UIDs.
At this stage, we really don't assume any ordering of the clauses, nor try to
modify its ordering yet, so the base-pointer map (if it exists) could be any
where in the list (building some "visited set" isn't really suitable here).
I don't think this is really that much an issue of concern though.
+ if (OMP_CLAUSE_CODE (m) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_DECL (m) == ptr
Does it really need to be equality? I mean it will be for
map(tofrom:ptr) map(tofrom:ptr[:32])
but what about e.g.
map(tofrom:structx) map(tofrom:structx.ptr[:32])
? It is true that likely we don't parse this yet though.
The code for COMPONENT_REF based expressions are actually handled quite
differently
in gimplify_scan_omp_clauses. Not completely sure there's nothing to handle for
the
code in this patch set, but will have to discover such testcases later.
+ && (OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALLOC
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TO
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_FROM
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TOFROM))
What about the always modified mapping kinds?
Took care of that.
+ {
+ ptr_mapped = true;
+ break;
+ }
+
+ if (!ptr_mapped
+ && DECL_P (ptr)
+ && is_global_var (ptr)
+ && lookup_attribute ("omp declare target",
+DECL_ATTRIBUTES (ptr)))
+ ptr_mapped = true;
+ }
+
+ /* If the pointer variable was mapped, or if this is not an offloaded
+ target region, adjust the map kind to attach/detach. */
+ if (ptr_mapped || !is_target)
+ {
+ OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_ATTACH_DETACH);
+ c_common_mark_addressable_vec (ptr);
Though perhaps this is argument why it needs to be done in the FEs and not
during gimplification, because it is hard to mark something addressable at
that point.
Discussed above.
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -13580,16 +13580,17 @@ handle_omp_array_sections (tree c, enum
c_omp_region_type ort)
break;
}
tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP);
if (ort != C_ORT_OMP && ort != C_ORT_ACC)
OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_POINTER);
else if (TREE_CODE (t) == COMPONENT_REF)
{
- gomp_map_kind k = (ort == C_ORT_ACC) ? GOMP_MAP_ATTACH_DETACH
- : GOMP_MAP_ALWAYS_POINTER;
+
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
On Tue, Sep 01, 2020 at 09:16:23PM +0800, Chung-Lin Tang wrote:
> this patch set implements parts of the target mapping changes introduced
> in OpenMP 5.0, mainly the attachment requirements for pointer-based
> list items, and the clause ordering.
>
> The first patch here are the C/C++ front-end changes.
>
> The entire set of changes has been tested for without regressions for
> the compiler and libgomp. Hope this is ready to commit to master.
Sorry for the delay in patch review and thanks for the standard citations,
that really helps.
> gcc/c-family/
> * c-common.h (c_omp_adjust_clauses): New declaration.
> * c-omp.c (c_omp_adjust_clauses): New function.
Besides the naming, I wonder why is it done in a separate function and so
early, can't what the function does be done either in
{,c_}finish_omp_clauses (provided we'd pass separate ORT_OMP vs.
ORT_OMP_TARGET to it to determine if it is target region vs. anything else),
or perhaps even better during gimplification (gimplify_scan_omp_clauses)?
> gcc/c/
> * c-parser.c (c_parser_omp_target_data): Add use of
> new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as
> handled map clause kind.
> (c_parser_omp_target_enter_data): Likewise.
> (c_parser_omp_target_exit_data): Likewise.
> (c_parser_omp_target): Likewise.
> * c-typeck.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
> use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type.
> (c_finish_omp_clauses): Adjust bitmap checks to allow struct decl and
> same struct field access to co-exist on OpenMP construct.
>
> gcc/cp/
> * parser.c (cp_parser_omp_target_data): Add use of
> new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as
> handled map clause kind.
> (cp_parser_omp_target_enter_data): Likewise.
> (cp_parser_omp_target_exit_data): Likewise.
> (cp_parser_omp_target): Likewise.
> * semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
> use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix
> interaction between reference case and attach/detach.
> (finish_omp_clauses): Adjust bitmap checks to allow struct decl and
> same struct field access to co-exist on OpenMP construct.
The changelog has some 8 space indented lines.
> + for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> +if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
> + && TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
> + {
> + tree ptr = OMP_CLAUSE_DECL (c);
> + bool ptr_mapped = false;
> + if (is_target)
> + {
> + for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
Isn't this O(n^2) in number of clauses? I mean, e.g. for the equality
comparisons (but see below) it could be dealt with e.g. using some bitmap
with DECL_UIDs.
> + if (OMP_CLAUSE_CODE (m) == OMP_CLAUSE_MAP
> + && OMP_CLAUSE_DECL (m) == ptr
Does it really need to be equality? I mean it will be for
map(tofrom:ptr) map(tofrom:ptr[:32])
but what about e.g.
map(tofrom:structx) map(tofrom:structx.ptr[:32])
? It is true that likely we don't parse this yet though.
> + && (OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALLOC
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TO
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_FROM
> + || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TOFROM))
What about the always modified mapping kinds?
> + {
> + ptr_mapped = true;
> + break;
> + }
> +
> + if (!ptr_mapped
> + && DECL_P (ptr)
> + && is_global_var (ptr)
> + && lookup_attribute ("omp declare target",
> + DECL_ATTRIBUTES (ptr)))
> + ptr_mapped = true;
> + }
> +
> + /* If the pointer variable was mapped, or if this is not an offloaded
> +target region, adjust the map kind to attach/detach. */
> + if (ptr_mapped || !is_target)
> + {
> + OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_ATTACH_DETACH);
> + c_common_mark_addressable_vec (ptr);
Though perhaps this is argument why it needs to be done in the FEs and not
during gimplification, because it is hard to mark something addressable at
that point.
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -13580,16 +13580,17 @@ handle_omp_array_sections (tree c, enum
> c_omp_region_type ort)
> break;
> }
>tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP);
>if (ort != C_ORT_OMP && ort != C_ORT_ACC)
> OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_POINTER);
>else if (TREE_CODE (t) == COMPONENT_REF)
> {
> - gomp_map_kind k = (ort == C_ORT_ACC) ? GOMP_MAP_ATTACH_DETACH
> -
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
On 2020/9/29 6:16 PM, Jakub Jelinek wrote:
On Tue, Sep 01, 2020 at 09:16:23PM +0800, Chung-Lin Tang wrote:
this patch set implements parts of the target mapping changes introduced
in OpenMP 5.0, mainly the attachment requirements for pointer-based
list items, and the clause ordering.
The first patch here are the C/C++ front-end changes.
Do you think you could mention in detail which exact target mapping changes
in the spec is the patchset attempting to implement?
5.0 unfortunately contains many target mapping changes and this patchset
can't implement them all and it would be easier to see the list of rules
(e.g. from openmp-diff-full-4.5-5.0.pdf, if you don't have that one, I can
send it to you), rather than trying to guess them from the patchset.
Thanks.
Hi Jakub,
the main implemented features are the clause ordering rules:
"For a given construct, the effect of a map clause with the to, from, or
tofrom map-type is
ordered before the effect of a map clause with the alloc, release, or delete
map-type."
"If item1 is a list item in a map clause, and item2 is another list item in a
map clause on
the same construct that has a base pointer that is, or is part of, item1,
then:
* If the map clause(s) appear on a target, target data, or target enter
data construct,
then on entry to the corresponding region the effect of the map clause on
item1 is ordered
to occur before the effect of the map clause on item2.
* If the map clause(s) appear on a target, target data, or target exit
data construct then
on exit from the corresponding region the effect of the map clause on
item2 is ordered to
occur before the effect of the map clause on item1."
and the base-pointer attachment behavior:
"If a list item in a map clause has a base pointer, and a pointer variable is
present in the device data
environment that corresponds to the base pointer when the effect of the map
clause occurs, then if
the corresponding pointer or the corresponding list item is created in the
device data environment
on entry to the construct, then:
...
2. The corresponding pointer variable becomes an attached pointer for the
corresponding list item."
(these passages are all in the "2.19.7.1 map Clause" section of the 5.0 spec,
all are new as
also verified from the diff PDFs you sent us)
Also, because of the these new features, having multiple maps of the same
variables now have meaning
in OpenMP, so changes in the C/C++ frontends to relax the no-duplicate rules
are also included.
gcc/c-family/
* c-common.h (c_omp_adjust_clauses): New declaration.
* c-omp.c (c_omp_adjust_clauses): New function.
This function name is too broad, it should have target in it as it is
for processing target* construct clauses only.
Jakub
Sure, I'll update this naming in a later version.
Thanks,
Chung-Lin
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
On Tue, Sep 01, 2020 at 09:16:23PM +0800, Chung-Lin Tang wrote: > this patch set implements parts of the target mapping changes introduced > in OpenMP 5.0, mainly the attachment requirements for pointer-based > list items, and the clause ordering. > > The first patch here are the C/C++ front-end changes. Do you think you could mention in detail which exact target mapping changes in the spec is the patchset attempting to implement? 5.0 unfortunately contains many target mapping changes and this patchset can't implement them all and it would be easier to see the list of rules (e.g. from openmp-diff-full-4.5-5.0.pdf, if you don't have that one, I can send it to you), rather than trying to guess them from the patchset. Thanks. > gcc/c-family/ > * c-common.h (c_omp_adjust_clauses): New declaration. > * c-omp.c (c_omp_adjust_clauses): New function. This function name is too broad, it should have target in it as it is for processing target* construct clauses only. Jakub
Re: [PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
Ping this patch set. Thanks, Chung-Lin On 2020/9/1 9:16 PM, Chung-Lin Tang wrote: Hi Jakub, this patch set implements parts of the target mapping changes introduced in OpenMP 5.0, mainly the attachment requirements for pointer-based list items, and the clause ordering. The first patch here are the C/C++ front-end changes. The entire set of changes has been tested for without regressions for the compiler and libgomp. Hope this is ready to commit to master. Thanks, Chung-Lin gcc/c-family/ * c-common.h (c_omp_adjust_clauses): New declaration. * c-omp.c (c_omp_adjust_clauses): New function. gcc/c/ * c-parser.c (c_parser_omp_target_data): Add use of new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as handled map clause kind. (c_parser_omp_target_enter_data): Likewise. (c_parser_omp_target_exit_data): Likewise. (c_parser_omp_target): Likewise. * c-typeck.c (handle_omp_array_sections): Adjust COMPONENT_REF case to use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. (c_finish_omp_clauses): Adjust bitmap checks to allow struct decl and same struct field access to co-exist on OpenMP construct. gcc/cp/ * parser.c (cp_parser_omp_target_data): Add use of new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as handled map clause kind. (cp_parser_omp_target_enter_data): Likewise. (cp_parser_omp_target_exit_data): Likewise. (cp_parser_omp_target): Likewise. * semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix interaction between reference case and attach/detach. (finish_omp_clauses): Adjust bitmap checks to allow struct decl and same struct field access to co-exist on OpenMP construct.
[PATCH, 1/3, OpenMP] Target mapping changes for OpenMP 5.0, front-end parts
Hi Jakub,
this patch set implements parts of the target mapping changes introduced
in OpenMP 5.0, mainly the attachment requirements for pointer-based
list items, and the clause ordering.
The first patch here are the C/C++ front-end changes.
The entire set of changes has been tested for without regressions for
the compiler and libgomp. Hope this is ready to commit to master.
Thanks,
Chung-Lin
gcc/c-family/
* c-common.h (c_omp_adjust_clauses): New declaration.
* c-omp.c (c_omp_adjust_clauses): New function.
gcc/c/
* c-parser.c (c_parser_omp_target_data): Add use of
new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as
handled map clause kind.
(c_parser_omp_target_enter_data): Likewise.
(c_parser_omp_target_exit_data): Likewise.
(c_parser_omp_target): Likewise.
* c-typeck.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type.
(c_finish_omp_clauses): Adjust bitmap checks to allow struct decl and
same struct field access to co-exist on OpenMP construct.
gcc/cp/
* parser.c (cp_parser_omp_target_data): Add use of
new c_omp_adjust_clauses function. Add GOMP_MAP_ATTACH_DETACH as
handled map clause kind.
(cp_parser_omp_target_enter_data): Likewise.
(cp_parser_omp_target_exit_data): Likewise.
(cp_parser_omp_target): Likewise.
* semantics.c (handle_omp_array_sections): Adjust COMPONENT_REF case to
use GOMP_MAP_ATTACH_DETACH map kind for C_ORT_OMP region type. Fix
interaction between reference case and attach/detach.
(finish_omp_clauses): Adjust bitmap checks to allow struct decl and
same struct field access to co-exist on OpenMP construct.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 4fc64bc4aa6..9ef85b401f0 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1208,14 +1208,15 @@ extern tree c_omp_declare_simd_clauses_to_numbers
(tree, tree);
extern void c_omp_declare_simd_clauses_to_decls (tree, tree);
extern bool c_omp_predefined_variable (tree);
extern enum omp_clause_default_kind c_omp_predetermined_sharing (tree);
extern enum omp_clause_defaultmap_kind c_omp_predetermined_mapping (tree);
extern tree c_omp_check_context_selector (location_t, tree);
extern void c_omp_mark_declare_variant (location_t, tree, tree);
extern const char *c_omp_map_clause_name (tree, bool);
+extern void c_omp_adjust_clauses (tree, bool);
/* Return next tree in the chain for chain_next walking of tree nodes. */
static inline tree
c_tree_chain_next (tree t)
{
/* TREE_CHAIN of a type is TYPE_STUB_DECL, which is different
kind of object, never a long chain of nodes. Prefer
diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index d7cff0f4cca..596f33cebfb 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -2575,7 +2575,51 @@ c_omp_map_clause_name (tree clause, bool oacc)
case GOMP_MAP_DEVICE_RESIDENT: return "device_resident";
case GOMP_MAP_LINK: return "link";
case GOMP_MAP_FORCE_DEVICEPTR: return "deviceptr";
default: break;
}
return omp_clause_code_name[OMP_CLAUSE_CODE (clause)];
}
+
+/* Adjust map clauses after normal clause parsing, mainly to turn specific
+ base-pointer map cases into attach/detach and mark them addressable. */
+void
+c_omp_adjust_clauses (tree clauses, bool is_target)
+{
+ for (tree c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
+if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
+ && TREE_CODE (TREE_TYPE (OMP_CLAUSE_DECL (c))) != ARRAY_TYPE)
+ {
+ tree ptr = OMP_CLAUSE_DECL (c);
+ bool ptr_mapped = false;
+ if (is_target)
+ {
+ for (tree m = clauses; m; m = OMP_CLAUSE_CHAIN (m))
+ if (OMP_CLAUSE_CODE (m) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_DECL (m) == ptr
+ && (OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_ALLOC
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TO
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_FROM
+ || OMP_CLAUSE_MAP_KIND (m) == GOMP_MAP_TOFROM))
+ {
+ ptr_mapped = true;
+ break;
+ }
+
+ if (!ptr_mapped
+ && DECL_P (ptr)
+ && is_global_var (ptr)
+ && lookup_attribute ("omp declare target",
+DECL_ATTRIBUTES (ptr)))
+ ptr_mapped = true;
+ }
+
+ /* If the pointer variable was mapped, or if this is not an offloaded
+ target region, adjust the map kind to attach/detach. */
+ if (ptr_mapped || !is_target)
+ {
+ OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_ATTACH_DETACH);
+ c_common_mark_addressable_vec (ptr);
+
