From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id AB0EF383B681; Wed, 7 Dec 2022 16:31:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AB0EF383B681 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="5.96,225,1665475200"; d="scan'208";a="88852187" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 07 Dec 2022 08:31:29 -0800 IronPort-SDR: FW3puNwFwZcTYTWY3Zaj+ZBXEkUKmG4wZ9MRxZ6Bphw0YkdexRnjWiLx3bqz9jMtp9xUvO8AMJ z5iJS7mbJvh1dOZ5AfESjpWjzv0wZciL7flyAZyRHGvtFIWlTFn1aaczfU22EfmFWw3gHI+qrI AASac3bl/54LIfaujO93xxD31uvhOJVy8EaV5ryHLKGGXeOsBu2JY2XV0qzYIW2EeP2c/R36QH 9Ym3brYLOtYcVSPA9Rfr081dX7ptWlH/9GHqTXRIiuVuz3TwwhUehyogpT/YQdguivO5noWITD m6E= Message-ID: Date: Wed, 7 Dec 2022 17:31:20 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH v5 3/4] OpenMP: Pointers and member mappings Content-Language: en-US To: Julian Brown , CC: , Jakub Jelinek References: <80f87c37a4f8b9f1f61c1668ecb750cefb1aec77.1666088224.git.julian@codesourcery.com> From: Tobias Burnus In-Reply-To: <80f87c37a4f8b9f1f61c1668ecb750cefb1aec77.1666088224.git.julian@codesourcery.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) To svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham 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 Julian, I think this patch is OK; however, at least for gimplify.cc Jakub needs to = have a second look. As remarked for the 2/4 patch, I believe mapping 'map(tofrom: var%f(2:3))' = should work without explicitly mapping 'map(tofrom: var%f)' (=E2=86=92 [TR11 157:21-26] (approx. [5.2 154:22-27], [5.1 352:17-22], [5.0= 320:22-27]). =E2=86=92 https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608100.ht= ml (+ previously in the thread). Testing the patch, that seems to work fine (i.e. contrary to C/C++, cf. 2/4= ), which matches the dump and, if I understood correctly, also your (Julian's)= expectation. Thus, no need to modify the code part. Regarding the testcases: * I would prefer if you don't modify the existing libgomp.fortran/struct-el= em-map-1.f90 testcase; However, you could add your version as another variant ('subroutine nine= ()', 'four_var()' or what's the next free name, possibly with a comment telling that it is 'f= our()' but with an added explicit basepointer mapping.). * As the new version should map *less*, I wonder whether some -fdump-tree-{= original,gimple,omplower} scan-dump-tree checks would be useful besides testing whether it works a= t run time. (Your decision regarding which tree, which testcases and whether at all.= ) * Likewise, maybe a 'target enter/exit data' check? However, you might very= well run into my 'omp target data exit' issue, cf. https://gcc.gnu.org/pipermail/gcc-patc= hes/2022-November/604887.html (needs to be revised based on Jakub's comments; I think those were on IR= C only =E2=80=93 the problem is that not only 'alloc' is affected but also 'from' etc.) On 18.10.22 12:39, Julian Brown wrote: > Implementing the "omp declare mapper" functionality, I noticed some > cases where handling of derived type members that are pointers doesn't > seem to be quite right. At present, a type such as this: > ... > map(to: tvar%arrptr) map(tofrom: tvar%arrptr(3:8)) > > and then instead we should follow (OpenMP 5.2, 5.8.3 "map Clause"): > ... > 2) map(tofrom: tvar%arrptr(3:8) --> > GOMP_MAP_TOFROM *tvar%arrptr%data(3) (size 8-3+1, etc.) > GOMP_MAP_TO_PSET tvar%arrptr > GOMP_MAP_ATTACH_DETACH tvar%arrptr%data (bias 3, etc.) > > ... > Additionally, the next patch in the series adds a runtime diagnostic > for the (illegal) case where 'i' and 'j' are different. > > 2022-10-18 Julian Brown > > gcc/fortran/ > * dependency.cc (gfc_omp_expr_prefix_same): New function. > * dependency.h (gfc_omp_expr_prefix_same): Add prototype. > * gfortran.h (gfc_omp_namelist): Add "duplicate_of" field to "u2" > union. > * trans-openmp.cc (dependency.h): Include. > (gfc_trans_omp_array_section): Use GOMP_MAP_TO_PSET unconditionally= for > mapping array descriptors. > (gfc_symbol_rooted_namelist): New function. > (gfc_trans_omp_clauses): Check subcomponent and subarray/element > accesses elsewhere in the clause list for pointers to derived types= or > array descriptors, and adjust or drop mapping nodes appropriately. > > gcc/ > * gimplify.cc (omp_tsort_mapping_groups): Process nodes that have > OMP_CLAUSE_MAP_RUNTIME_IMPLICIT_P set after those that don't. > (omp_accumulate_sibling_list): Adjust GOMP_MAP_TO_PSET handling. > Remove GOMP_MAP_ALWAYS_POINTER handling. > > libgomp/ > * testsuite/libgomp.fortran/map-subarray.f90: New test. > * testsuite/libgomp.fortran/map-subarray-2.f90: New test. > * testsuite/libgomp.fortran/map-subarray-3.f90: New test. > * testsuite/libgomp.fortran/map-subarray-4.f90: New test. > * testsuite/libgomp.fortran/map-subarray-6.f90: New test. > * testsuite/libgomp.fortran/map-subarray-7.f90: New test. > * testsuite/libgomp.fortran/map-subcomponents.f90: New test. > * testsuite/libgomp.fortran/struct-elem-map-1.f90: Adjust for > descriptor-mapping changes. Remove XFAIL. ... > --- a/libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90 > +++ b/libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90 > @@ -229,7 +229,8 @@ contains > > ! !$omp target map(tofrom: var%d(4:7), var%f(2:3), var%str2(2:3)) & > ! !$omp& map(tofrom: var%str4(2:2), var%uni2(2:3), var%uni4(2:2= )) > - !$omp target map(tofrom: var%d(4:7), var%f(2:3), var%str2(2:3), var%= uni2(2:3)) > + !$omp target map(to: var%f) map(tofrom: var%d(4:7), var%f(2:3), & > + !$omp& var%str2(2:3), var%uni2(2:3)) This adds 'to: var%f' (to the existing 'var%f(2:3)') =E2=80=93 where 'f' i= s a POINTER. As discussed at the top, I prefer to leave it as is =E2=80=93 and possibly just add another test-function, replicating this function and only there adding the basepointer as additional list item. > - !$omp target map(tofrom: var%f(2:3)) > + !$omp target map(to: var%f) map(tofrom: var%f(2:3)) likewise. > - !$omp target map(tofrom: var%d(5), var%f(3), var%str2(3), var%uni2(3= )) > + !$omp target map(to: var%f) map(tofrom: var%d(5), var%f(3), & > + !$omp& var%str2(3), var%uni2(3)) likewise. > - !$omp target map(tofrom: var%f(2:3)) > + !$omp target map(to: var%f) map(tofrom: var%f(2:3)) likewise. Thanks, Tobias ----------------- 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