From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id 743293858C33; Wed, 3 May 2023 13:51:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 743293858C33 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.99,247,1677571200"; d="scan'208";a="4204172" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa3.mentor.iphmx.com with ESMTP; 03 May 2023 05:50:55 -0800 IronPort-SDR: tRrsIapxr10S7lp+xPEFEHmQXifArMLfYIrJVzw/firTMQPYO4aXm9ypRLi5w2azj6nBtfvcqS T1kzP7jkiI1lhaiSBGDbl2P2tw3kLkM3s+AZA9TvSpSBihyp0UPb6KQRPX2h6nUoE9fQi3dFx9 P8TsRPvZw6cVuS8QmaGFkce0hsP8xM5i8kEVyHGU8Ne2THw0Bl/U2ZpXO9u5IERoiX8OijvhvH ZbjqgMLD5msvpLkpoBq2Hwg9nhUSGhUzqMF9nj/fwQA/+XDAD4bqagjwr3V0d/OfXEWaZsTpsd W9g= Message-ID: Date: Wed, 3 May 2023 15:50:49 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: [PATCH] OpenACC: Further attach/detach clause fixes for Fortran [PR109622] Content-Language: en-US To: Julian Brown CC: , , , References: <20230429105741.108576-1-julian@codesourcery.com> <0af16eab-5b25-b167-ad4b-54612825d0ca@codesourcery.com> <20230503135956.1ec395a2@squid.athome> From: Tobias Burnus In-Reply-To: <20230503135956.1ec395a2@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-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=-13.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: On 03.05.23 14:59, Julian Brown wrote: > How does this version look? > Retested with offloading to nvptx. LGTM (for mainline + GCC 13 backporting) but I have two tiny comments: > diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc > index 86e4515..322856a 100644 > --- a/gcc/fortran/openmp.cc > +++ b/gcc/fortran/openmp.cc > @@ -7711,6 +7711,23 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_claus= es *omp_clauses, > &n->where); > } > } > + if (openacc > + && list =3D=3D OMP_LIST_MAP > + && (n->u.map_op =3D=3D OMP_MAP_ATTACH > + || n->u.map_op =3D=3D OMP_MAP_DETACH)) > + { > + symbol_attribute attr; > + gfc_clear_attr (&attr); > + if (n->expr) > + attr =3D gfc_expr_attr (n->expr); > + else if (n->sym) > + attr =3D n->sym->attr; Note that n->sym =3D=3D NULL is only the case if the argument was omp_all_memory (=E2=86=92 gfc_match_omp_variable_list); that can only be th= e case for OMP_CLAUSE_DEPEND. As OpenMP's 'depend' clause is handled before and you additionally deal with OpenACC, only, you could just use 'else' instead of 'else if (n->sym)' =E2=80=93 and also get rid of the 'gfc_clear_attr' as the values = get overwritten by the assignment and by the function call. Later code (e.g. line 7785 in the current code) also assumes (for OpenACC + MAP) that n->sym !=3D NULL by bluntly dereferencing it. @@ -3523,15 +3525,20 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses, > if (n->u.map_op =3D=3D OMP_MAP_ATTACH > || n->u.map_op =3D=3D OMP_MAP_DETACH) > { > - if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (inner))) > + if (POINTER_TYPE_P (TREE_TYPE (inner)) > + || GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (inner))) > { ... > } > - else > - OMP_CLAUSE_DECL (node) =3D inner; > - OMP_CLAUSE_SIZE (node) =3D size_zero_node; > - goto finalize_map_clause; > } You can now combine the two if conditions, which avoids some indenting and should permit to put 'tree ptr' / ' =3D ...' again on the same line. Thanks for the patch! 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