From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id D67F23892441 for ; Mon, 14 Jun 2021 16:18:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D67F23892441 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-495-lUB_zduLNDyrLhN0Z58BiA-1; Mon, 14 Jun 2021 12:18:18 -0400 X-MC-Unique: lUB_zduLNDyrLhN0Z58BiA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 10387107ACF6; Mon, 14 Jun 2021 16:18:17 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-7.ams2.redhat.com [10.36.112.7]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A0ABE61093; Mon, 14 Jun 2021 16:18:16 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 15EGIDTC1833042 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 14 Jun 2021 18:18:14 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 15EGICnj1833041; Mon, 14 Jun 2021 18:18:12 +0200 Date: Mon, 14 Jun 2021 18:18:12 +0200 From: Jakub Jelinek To: Tobias Burnus Cc: gcc-patches , fortran Subject: Re: [Patch ]Fortran/OpenMP: Extend defaultmap clause for OpenMP 5 [PR92568] Message-ID: <20210614161812.GH7746@tucnak> Reply-To: Jakub Jelinek References: <90ebbf24-6d0f-7964-0586-e5fc4f615b40@codesourcery.com> MIME-Version: 1.0 In-Reply-To: <90ebbf24-6d0f-7964-0586-e5fc4f615b40@codesourcery.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Jun 2021 16:18:25 -0000 On Wed, Jun 09, 2021 at 02:18:43PM +0200, Tobias Burnus wrote: > This patch add's OpenMP 5.1's defaultmap extensions to Fortran. > > There is one odd thing, > integer :: ii, it > target :: it > both count as nonallocatable, nonpointer scalars (i.e. category 'scalar'). > But with implicit mapping (and 'defaultmap(default)'), 'it' is mapped > tofrom due to the TARGET attribute (cf. quote in the PR). The list in 5.1 2.21.7 is ordered, so if defaultmap is present and is not default, it takes precedence over TARGET attribute. So, it above with defaultmap(firstprivate:scalar) will result in firstprivate(it, ii), while no defaultmap or default for it will result in map(tofrom: it) firstprivate (ii) (same for ALLOCATABLE/POINTER). > + switch ((enum gfc_omp_defaultmap) i) > + { > + case OMP_DFLTMAP_CAT_SCALAR: dfltmap = "SCALAR"; break; > + case OMP_DFLTMAP_CAT_AGGREGATE: dfltmap = "AGGREGATE"; break; > + case OMP_DFLTMAP_CAT_ALLOCATABLE: dfltmap = "ALLOCATABLE"; break; > + case OMP_DFLTMAP_CAT_POINTER: dfltmap = "POINTER"; break; > + default: gcc_unreachable (); Formatting. case/default should be indented the same as {. > --- a/gcc/fortran/gfortran.h > +++ b/gcc/fortran/gfortran.h > @@ -1241,6 +1241,29 @@ enum gfc_omp_map_op > OMP_MAP_ALWAYS_TOFROM > }; > > +enum gfc_omp_defaultmap > +{ > + OMP_DFLTMAP_UNSET, > + OMP_DFLTMAP_ALLOC, > + OMP_DFLTMAP_TO, > + OMP_DFLTMAP_FROM, > + OMP_DFLTMAP_TOFROM, > + OMP_DFLTMAP_FIRSTPRIVATE, > + OMP_DFLTMAP_NONE, > + OMP_DFLTMAP_DEFAULT, > + OMP_DFLTMAP_PRESENT Any reason not to use full OMP_DEFAULTMAP_ ? The extra 3 chars will improve readability I think. > +}; > + > +enum gfc_omp_dfltmpap_category Was this meant to be dfltmap rather than mpap? I think I'd prefer omp_defaultmap_category > +{ > + OMP_DFLTMAP_CAT_UNCATEGORIZED, > + OMP_DFLTMAP_CAT_SCALAR, > + OMP_DFLTMAP_CAT_AGGREGATE, > + OMP_DFLTMAP_CAT_ALLOCATABLE, > + OMP_DFLTMAP_CAT_POINTER, > + OMP_DFLTMAP_CAT_NUM And same as above. > + switch(clauses->defaultmap[i]) Missing space after switch. > diff --git a/gcc/testsuite/gfortran.dg/gomp/defaultmap-1.f90 b/gcc/testsuite/gfortran.dg/gomp/defaultmap-1.f90 > new file mode 100644 > index 00000000000..299d971f23c As for the testsuite, I miss the integer, target :: it case in the gfortran.dg/gomp/defaultmap-*.f90 tests, it is only in the runtime case if I'm not blind. Jakub