Hi Tobias! On 2023-06-12T18:44:23+0200, Tobias Burnus <tob...@codesourcery.com> wrote: > Cleanup follow up to > r14-1579-g4ede915d5dde93 "openmp: Add support for the 'present' modifier" > committed 6 days ago. > > Namely: > * Replace for the program → libgomp ABI > GOMP_MAP_PRESENT_[ALLOC,TO,FROM,TOFROM] > by the preexisting GOMP_MAP_FORCE_PRESENT but keep the other enum values > (and use them until gimplifcation). > > * Improve wording if a non-existing/unsupported map-type modifier was used > by not referring to 'omp target' as it could be also target (enter/exit) > data. > + Add a testcase for enter/exit data + data. > > * Unify + improve wording shown for 'present' when not present on the device. > > * Extend in the testcases to check that data actually gets copied with > 'target update' and 'map when the 'present' modifier is present. > > Committed as Rev. r14-1736-g38944ec2a6fa10
> OpenMP: Cleanups related to the 'present' modifier > > Reduce number of enum values passed to libgomp as > GOMP_MAP_PRESENT_{TO,TOFROM,FROM,ALLOC} have the same semantic as > GOMP_MAP_FORCE_PRESENT (i.e. abort if not present, otherwise ignore); > that's different to GOMP_MAP_ALWAYS_PRESENT_{TO,TOFROM,FROM} which also > abort if not present but copy data when present. This is is a follow-up to > the commit r14-1579-g4ede915d5dde93 done 6 days ago. Great, that matches how I thought this should be done (re our 2023-06-07 GCC IRC discussion). > Additionally, the commit [...] > extends testcases a tiny bit. > gcc/testsuite/ChangeLog: > * gfortran.dg/gomp/target-update-1.f90: Likewise. That one addressed fixed <https://gcc.gnu.org/110178> "gfortran.dg/gomp/target-update-1.f90 fails after r14-1579-g4ede915d5dde93". > --- a/include/gomp-constants.h > +++ b/include/gomp-constants.h | #define GOMP_MAP_FLAG_PRESENT (GOMP_MAP_FLAG_SPECIAL_5 \ | | GOMP_MAP_FLAG_SPECIAL_0) Couldn't/shouldn't we now get rid of this 'GOMP_MAP_FLAG_PRESENT'... | #define GOMP_MAP_FLAG_ALWAYS_PRESENT (GOMP_MAP_FLAG_SPECIAL_2 \ | | GOMP_MAP_FLAG_PRESENT) ..., as it is only used in 'GOMP_MAP_FLAG_ALWAYS_PRESENT' here... > @@ -136,14 +136,6 @@ enum gomp_map_kind > device. */ > GOMP_MAP_ALWAYS_TOFROM = (GOMP_MAP_FLAG_SPECIAL_2 > | GOMP_MAP_TOFROM), > - /* Must already be present. */ > - GOMP_MAP_PRESENT_ALLOC = (GOMP_MAP_FLAG_PRESENT | > GOMP_MAP_ALLOC), > - /* Must already be present, copy to device. */ > - GOMP_MAP_PRESENT_TO = (GOMP_MAP_FLAG_PRESENT | GOMP_MAP_TO), > - /* Must already be present, copy from device. */ > - GOMP_MAP_PRESENT_FROM = (GOMP_MAP_FLAG_PRESENT | GOMP_MAP_FROM), > - /* Must already be present, copy to and from device. */ > - GOMP_MAP_PRESENT_TOFROM = (GOMP_MAP_FLAG_PRESENT | > GOMP_MAP_TOFROM), > /* Must already be present, unconditionally copy to device. */ > GOMP_MAP_ALWAYS_PRESENT_TO = (GOMP_MAP_FLAG_ALWAYS_PRESENT > | GOMP_MAP_TO), > @@ -205,7 +197,13 @@ enum gomp_map_kind > /* An attach or detach operation. Rewritten to the appropriate type > during > gimplification, depending on directive (i.e. "enter data" or > parallel/kernels region vs. "exit data"). */ > - GOMP_MAP_ATTACH_DETACH = (GOMP_MAP_LAST | 3) > + GOMP_MAP_ATTACH_DETACH = (GOMP_MAP_LAST | 3), > + /* Must already be present - all of following map to > GOMP_MAP_FORCE_PRESENT > + as no data transfer is needed. */ > + GOMP_MAP_PRESENT_ALLOC = (GOMP_MAP_LAST | 4), > + GOMP_MAP_PRESENT_TO = (GOMP_MAP_LAST | 5), > + GOMP_MAP_PRESENT_FROM = (GOMP_MAP_LAST | 6), > + GOMP_MAP_PRESENT_TOFROM = (GOMP_MAP_LAST | 7) > }; > > #define GOMP_MAP_COPY_TO_P(X) \ > @@ -243,7 +241,8 @@ enum gomp_map_kind > (((X) & GOMP_MAP_FLAG_SPECIAL_BITS) == GOMP_MAP_FLAG_FORCE) > > #define GOMP_MAP_PRESENT_P(X) \ > - (((X) & GOMP_MAP_FLAG_PRESENT) == GOMP_MAP_FLAG_PRESENT) > + (((X) & GOMP_MAP_FLAG_PRESENT) == GOMP_MAP_FLAG_PRESENT \ > + || (X) == GOMP_MAP_FORCE_PRESENT) ..., and this 'GOMP_MAP_PRESENT_P' should look for 'GOMP_MAP_FLAG_ALWAYS_PRESENT' instead of 'GOMP_MAP_FLAG_PRESENT' (plus 'GOMP_MAP_FORCE_PRESENT')? Instead of the current effective 'GOMP_MAP_FLAG_ALWAYS_PRESENT': GOMP_MAP_FLAG_SPECIAL_0 | GOMP_MAP_FLAG_SPECIAL_2 | GOMP_MAP_FLAG_SPECIAL_5 ..., it could/should use a simpler flag combination? (My idea is that this later make usage of flag bits for other purposes easier -- but I've not verified that in depth.) Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955