From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id F2DE93858D38 for ; Wed, 14 Jun 2023 08:42:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F2DE93858D38 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="6.00,242,1681200000"; d="scan'208";a="9817503" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 14 Jun 2023 00:42:34 -0800 IronPort-SDR: j6sB96t+4YLuf+WYsgu33TzzksUSjk9WNnLeczAz4olJeTxWSC4m/E39pnBAd8/WsksyToNdPp SxXgtws0zdo/GyeFiit5W2V3JrD+dQdgzquAFfgXqLAcDAZQjQAz63pi/0Z0OOKS4NcLDQpjGJ jIXyRz2GyWpvSsulRK2i/hwk5ImsocT9gdjIE6pRAZ09zzVxchx8pgZOhTmSAu3iXXlEJaUVHj eS3+iOtti6w0M3XsIkj7NkZ2dtWgHHvjN0qpkhHPo/NpTW1feBVS+Td8C868TcJCzeuqQsLAP3 TWs= From: Thomas Schwinge To: Tobias Burnus , CC: Jakub Jelinek , Kwok Cheung Yeung Subject: Re: [committed] OpenMP: Cleanups related to the 'present' modifier In-Reply-To: <64017201-8206-fd22-70e4-897c858ae049@codesourcery.com> References: <6eb5d0dd-da2a-6d8e-eaa2-d14bf708cf36@codesourcery.com> <049a4654-2596-1913-20fc-1aeea48eb3ec@codesourcery.com> <64017201-8206-fd22-70e4-897c858ae049@codesourcery.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Wed, 14 Jun 2023 10:42:27 +0200 Message-ID: <87jzw6305o.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Tobias! On 2023-06-12T18:44:23+0200, Tobias Burnus wrote: > Cleanup follow up to > r14-1579-g4ede915d5dde93 "openmp: Add support for the 'present' modifi= er" > committed 6 days ago. > > Namely: > * Replace for the program =E2=86=92 libgomp ABI GOMP_MAP_PRESENT_[ALLOC,T= O,FROM,TOFROM] > by the preexisting GOMP_MAP_FORCE_PRESENT but keep the other enum valu= es > (and use them until gimplifcation). > > * Improve wording if a non-existing/unsupported map-type modifier was use= d > by not referring to 'omp target' as it could be also target (enter/exi= t) data. > + Add a testcase for enter/exit data + data. > > * Unify + improve wording shown for 'present' when not present on the dev= ice. > > * 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 al= so > 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 "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 =3D (GOMP_MAP_FLAG_SPECIAL_2 > | GOMP_MAP_TOFROM), > - /* Must already be present. */ > - GOMP_MAP_PRESENT_ALLOC =3D (GOMP_MAP_FLAG_PRESENT | GOMP_MAP= _ALLOC), > - /* Must already be present, copy to device. */ > - GOMP_MAP_PRESENT_TO =3D (GOMP_MAP_FLAG_PRESENT | GOMP_MAP= _TO), > - /* Must already be present, copy from device. */ > - GOMP_MAP_PRESENT_FROM =3D (GOMP_MAP_FLAG_PRESENT | GOMP_MAP= _FROM), > - /* Must already be present, copy to and from device. */ > - GOMP_MAP_PRESENT_TOFROM =3D (GOMP_MAP_FLAG_PRESENT | = GOMP_MAP_TOFROM), > /* Must already be present, unconditionally copy to device. */ > GOMP_MAP_ALWAYS_PRESENT_TO =3D (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 =3D (GOMP_MAP_LAST | 3) > + GOMP_MAP_ATTACH_DETACH =3D (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 =3D (GOMP_MAP_LAST | 4), > + GOMP_MAP_PRESENT_TO =3D (GOMP_MAP_LAST | 5), > + GOMP_MAP_PRESENT_FROM =3D (GOMP_MAP_LAST | 6), > + GOMP_MAP_PRESENT_TOFROM =3D (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) =3D=3D GOMP_MAP_FLAG_FORCE) > > #define GOMP_MAP_PRESENT_P(X) \ > - (((X) & GOMP_MAP_FLAG_PRESENT) =3D=3D GOMP_MAP_FLAG_PRESENT) > + (((X) & GOMP_MAP_FLAG_PRESENT) =3D=3D GOMP_MAP_FLAG_PRESENT \ > + || (X) =3D=3D 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=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955