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 F3D4C3834869; Thu, 8 Dec 2022 12:05:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F3D4C3834869 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,227,1665475200"; d="scan'208";a="92116174" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 08 Dec 2022 04:05:10 -0800 IronPort-SDR: nKe3jsPZ0bGwe1gJUhzoucctftzapLZVKd1Q8TPr9Mg6tGmBr3eHQYi1uD7WF1kvoamdbPNFWK aj+SH02wkVg/Qob+2X3XwwFafVnwVFNxrksp+o4GnqeRdwsPrscCw7gMaJX1zHUrpUFXSXDegt DcWWGVQt1ziI/qKs+lHBX3TlURcPOC+Oeioapza8PilTnXFM2c6yb7sF49IL+CqGG+KAGR1JOO F8xtuMYHW1CuMdm1Kk2iQyOiCNYiiO3VpRMM86vYPHe5fYqoN1974k/XRqsWTRFhQNoA7bEOd5 EYo= Message-ID: Date: Thu, 8 Dec 2022 13:04: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 2/2] OpenMP: Duplicate checking for map clauses in Fortran (PR107214) Content-Language: en-US To: Julian Brown CC: , Jakub Jelinek , References: <20221020161414.7430-1-julian@codesourcery.com> <20221207190903.78a6b37f@squid.athome> <20221207191355.2e43ea14@squid.athome> From: Tobias Burnus In-Reply-To: <20221207191355.2e43ea14@squid.athome> 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-11.mgc.mentorg.com (139.181.222.11) 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,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: On 07.12.22 20:13, Julian Brown wrote: >> I know that this was the case before, but can you move the mark:1 etc. >> after 'tlink'? In that case all bitfields are grouped together. Thanks for doing so. >> I wonder whether that also rejects the following =E2=80=93 which seems t= o be >> valid. The 'map' goes to 'target' and the 'firstprivate' to >> 'parallel', cf. OpenMP 5.2, "17.2 Clauses on Combined and Composite >> Constructs", [340:3-4 & 12-14]. (BTW: While some fixes went into 5.1 >> regarding this section, a likewise wording is already in 5.0.) >> >> (Testing showed: it give an ICE without the patch and an error with.) > ...and this patch avoids the error for combined directives, and > reorders the gfc_symbol bitfields. All in all, I am fine with the patch - but I spotted some issues. First, I think you need to set for some error cases mark =3D 0 to avoid dup= licated errors. Namely: ! Outputs the error twice ('Symbol =E2=80=98y=E2=80=99 present on multip= le clauses') !$omp target has_device_addr(y) firstprivate(y) block; end block * * * Additionally, I think it would be good to have besides 'target' + map/first= private (=E2=86=92 error) also a testcase for 'target simd' + map/firstprivate =E2=86=92 error And I think also gives-no-error checks all combined 'target ...' that take = firstprivate should be added, cf. your own patch - possibly with looking at the original= dump (scan-tree-dump) to see that the clause is properly attached correctly. Example for 'target = teams': !$omp target teams map(x) firstprivate(x) block; end block (Works but no testcase.) * * * The following is not diagnosed and gives an ICE: !$omp target in_reduction(+: x) private(x) block; end block end The C testcase properly has: error: =E2=80=98x=E2=80=99 appears more than once in data-sharing clause= s Note: Using 'firstprivate' instead of 'private' shows the proper error also= in Fortran. The following does not ICE but does not make sense (and is rejected in C): 4 | #pragma omp target private(x) map(x) vs. !$omp target map(x) private(x) block; end block (The latter produces "#pragma omp target private(x.0) map(tofrom:*x.0)", up= s!) * * * I also note that 'simd' accepts private such that #pragma omp target simd private(x) map(x) for (int i=3D0; i < 0; i++) ; !$omp target simd map(x) private(x) do i =3D 1, 0; end do is valid. (It is accepted by gcc and gfortran, i.e. it just needs to be add= ed as testcase.) * * * I note that C rejects {map(x),firstprivate(x)} + {has_device_addr(x),is_dev= ice_ptr(x)}', but gfortran + your patch accepts: !$omp target map(x) has_device_addr(x) !$omp target map(x) is_device_ptr(x) while !$omp target firstprivate(x) has_device_addr(x) !$omp target firstprivate(x) is_device_ptr(x) is rejected =E2=80=93 showing the error message twice. Expected: I think it should show an error in all four cases - but only once= . > 2022-12-06 Julian Brown > > gcc/fortran/ > PR fortran/107214 > * gfortran.h (gfc_symbol): Add data_mark, dev_mark, gen_mark and > reduc_mark bitfields. > * openmp.cc (resolve_omp_clauses): Use above bitfields to improv= e > duplicate clause detection. > > gcc/testsuite/ > PR fortran/107214 > * gfortran.dg/gomp/pr107214.f90: New test. 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