| Message ID | 037fa9b35dfbe459776be8ef7b01eca95e3dc7a3.1592343756.git.julian@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | Refcounting and manual deep copy improvements | expand |
Hi Julian! On 2020-06-16T15:38:32-0700, Julian Brown <julian@codesourcery.com> wrote: > Later patches in this series assume that GOMP_MAP_ATTACH will be grouped > together with a preceding GOMP_MAP_TO_PSET or other "to" data movement > clause, except in cases where an explicit "attach" clause is used. > This patch arranges for that to be so. > > OK? OK for master branch and releases/gcc-10 branch. However, still a few questions, which can be addressed separately: > --- a/libgomp/oacc-mem.c > +++ b/libgomp/oacc-mem.c > @@ -985,9 +985,15 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) > switch (kind0) > { > case GOMP_MAP_TO_PSET: > - while (pos + 1 < mapnum && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) > + if (pos + 1 < mapnum > + && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) > + return pos + 1; > + > + while (pos + 1 < mapnum > + && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) > pos++; > - /* We expect at least one GOMP_MAP_POINTER after a GOMP_MAP_TO_PSET. */ > + /* We expect at least one GOMP_MAP_POINTER (if not a single > + GOMP_MAP_ATTACH) after a GOMP_MAP_TO_PSET. */ > assert (pos > first_pos); > break; So a 'GOMP_MAP_TO_PSET' can now be followed by either a single 'GOMP_MAP_ATTACH', or by potentially several 'GOMP_MAP_POINTER's, but not both. If the former ('GOMP_MAP_ATTACH'), then any additional following 'GOMP_MAP_POINTER's are not anymore handled together with the 'GOMP_MAP_TO_PSET' -- which defeats the description in 'include/gomp-constants.h', which explicitly details how 'GOMP_MAP_TO_PSET' is used to specially handle 'GOMP_MAP_POINTER's following it. So, please update the 'enum gomp_map_kind' definition to describe what's (now) actually going on. (Maybe that'll then make obsolete the source code comments you're adding here? ..., if also updating the 'GOMP_MAP_ATTACH' description to detail how it may follow 'GOMP_MAP_TO' etc. I think such rationale -- describing the valid combinations -- is better put there instead of into 'find_group_last'.) In the compiler, are we making sure that after 'GOMP_MAP_TO_PSET' we're not trying to emit both a 'GOMP_MAP_ATTACH' as well as 'GOMP_MAP_POINTER's? > @@ -1002,6 +1008,9 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) > gomp_fatal ("unexpected mapping"); > break; > > + case GOMP_MAP_ATTACH: > + return pos; > + > default: > /* GOMP_MAP_ALWAYS_POINTER can only appear directly after some other > mapping. */ > @@ -1012,9 +1021,16 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) > return pos + 1; > } > > + /* We can have a single GOMP_MAP_ATTACH mapping after a to/from > + mapping. */ > + if (pos + 1 < mapnum > + && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) > + return pos + 1; > + > /* We can have zero or more GOMP_MAP_POINTER mappings after a to/from > (etc.) mapping. */ > - while (pos + 1 < mapnum && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) > + while (pos + 1 < mapnum > + && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) > pos++; > } Similar (regarding documenting in 'enum gomp_map_kind' etc.). Grüße Thomas ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c index 936ae649dd9..be7f8d600eb 100644 --- a/libgomp/oacc-mem.c +++ b/libgomp/oacc-mem.c @@ -985,9 +985,15 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) switch (kind0) { case GOMP_MAP_TO_PSET: - while (pos + 1 < mapnum && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) + if (pos + 1 < mapnum + && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) + return pos + 1; + + while (pos + 1 < mapnum + && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) pos++; - /* We expect at least one GOMP_MAP_POINTER after a GOMP_MAP_TO_PSET. */ + /* We expect at least one GOMP_MAP_POINTER (if not a single + GOMP_MAP_ATTACH) after a GOMP_MAP_TO_PSET. */ assert (pos > first_pos); break; @@ -1002,6 +1008,9 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) gomp_fatal ("unexpected mapping"); break; + case GOMP_MAP_ATTACH: + return pos; + default: /* GOMP_MAP_ALWAYS_POINTER can only appear directly after some other mapping. */ @@ -1012,9 +1021,16 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) return pos + 1; } + /* We can have a single GOMP_MAP_ATTACH mapping after a to/from + mapping. */ + if (pos + 1 < mapnum + && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) + return pos + 1; + /* We can have zero or more GOMP_MAP_POINTER mappings after a to/from (etc.) mapping. */ - while (pos + 1 < mapnum && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) + while (pos + 1 < mapnum + && (kinds[pos + 1] & 0xff) == GOMP_MAP_POINTER) pos++; }