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 377F13857C55; Wed, 29 Nov 2023 16:03:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 377F13857C55 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 377F13857C55 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=68.232.129.153 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701273820; cv=none; b=XNTWSLh5J2UUZ5OJ+KmsmTVhqzIaQr35oZqb6QO/R1T9YdoTw5dr445BTGnpKzodkslur4nSpxy9fzCVHOQNXBnbX44OmG1L1QaKGegehy4L28ckYjcgBao92KLSJpEX7sHvQhlskhX0a0B3Q7QEC+5jeaGjwgsTDRJ33bBv58w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701273820; c=relaxed/simple; bh=JjjfBNm5AzbGg/62ylz0tN7Ciri3505jXpQXQ5a5IJ0=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=mhcgglzmjZa6AIsR9PeKtx81v8i7oLZJhX05ZA5QAiYL5Sm/RD77NfV6Ns/CbcC8XqPKG+xoHGN1R5B+MMj4pu7VKTNnzi6fo1XL1kkBnv9NQQs4rKAVed2eLs7lZwEQBN1yInpxQ4PGeab5coF/K3gfSl6sAsac9AfXtekcf0s= ARC-Authentication-Results: i=1; server2.sourceware.org X-CSE-ConnectionGUID: tU8joZy0RAGnfVMYC2DyrQ== X-CSE-MsgGUID: jjB45+8zQGu9xuiEux/EmA== X-IronPort-AV: E=Sophos;i="6.04,235,1695715200"; d="scan'208";a="27148554" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 29 Nov 2023 08:03:38 -0800 IronPort-SDR: BAezOyKGVNnxx/dIYTveejDddEzPVKBe4jCu4u0onPoJ/0pivDCPIOIqk+DlG4M3uhdc7Fdgjy lqhEC3FQYlH4zzafNfg5px6463r6fGvp2KQjjrYyylQYhxaNj02set/4BJM7K0qg6ND0MezyZs rhUUPBjUax+tuVOq8X/kKCrR0RN/QNHdrmoNUlQSEFSsx/NxohhtKdwgXz3lig0qB6dA3qPip2 j1uWH6SeRTAnPuEBiy74ayfdOjsDZN2dG7nooG8YridmRhcOZH/LRzzJmJl9EdViap8k/sGSsO /ko= Message-ID: Date: Wed, 29 Nov 2023 17:03:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 2/5] OpenMP/OpenACC: Rework clause expansion and nested struct handling Content-Language: en-US To: Julian Brown CC: , , References: <20231129114212.21f7b62d@squid.athome> From: Tobias Burnus In-Reply-To: <20231129114212.21f7b62d@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=-5.3 required=5.0 tests=BAYES_00,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,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 Julian, On 29.11.23 12:43, Julian Brown wrote: > Here is a patch incorporating your initial review comments > (hopefully!). Thanks. The patch LGTM - with the two remarks below addressed. (i.e. fixing one testcase and filing two PRs (or common PR) about the featu= res missing and exposed by the two test cases, referencing also to those testca= ses - and for the lvalues mentioning the OpenMP spec issue number.) * * * BTW: The 1/5 has been several times approved and is just reindenting - and is obviously still OK. (Review wise, 3/5, 4/5 and 5/5 still has to be done. I think the patch can go in before - given the huge improvements, even thou= gh it regresses for a few cases (xfail added for 2 Fortran testcases). 3/5 un-= xfails one and a half of the textcases, 5/5 un-xfails the remaining half and all o= f {3,4,5}/5 contain very useful improvements besides this. - But maybe waiting for at l= east 3/5 makes sense. In either case, I try to review the remaining patches soon.) * * * Question regarding the following: (a) The dg-xfail-run-if looks bogus as this an OpenMP test and not an OpenA= CC test (b) If there is shared memory, using 'omp target' should be fine. Namely, given that: > --- /dev/null +++ b/libgomp/testsuite/libgomp.c++/target-49.C @@ -0,0 > +1,37 @@ +#include +#include + +struct s { + int > (&a)[10]; + s(int (&a0)[10]) : a(a0) {} +}; + +int +main (int argc, > char *argv[]) +{ + int la[10]; + s v_real(la); + s *v =3D &v_real; + + > memset (la, 0, sizeof la); + + #pragma omp target enter data map(to: > v) + + /* Copying the whole v[0] here DOES NOT WORK yet because the > reference 'a' is + not copied "as if" it was mapped explicitly as a > member. FIXME. */ + #pragma omp target enter data map(to: v[0]) + + > //#pragma omp target + { + v->a[5]++; + } + + #pragma omp target exit > data map(release: v[0]) + #pragma omp target exit data map(from: v) + > + assert (v->a[5] =3D=3D 1); + + return 0; +} + +// { dg-xfail-run-if > "TODO" { *-*-* } { "-DACC_MEM_SHARED=3D0" } } Shouldn't the XFAIL not be based on '{ target offload_device_nonshared_as }= ' and the 'omp target' be uncommented? And I wonder whether we need to file a PR about this issue - I guess it is = not addressed by any of the follow-up issues and might get forgotten unless the= re is PR. * * * > libgomp/testsuite/libgomp.c++/baseptrs-4.C ... // Needs map clause > "lvalue"-parsing support. //#define REF2ARRAY_DECL_BASE There is an open OpenMP issue to disallow some lvalues, namely: OpenMP Issue 2618 ("Clarify behavior of mapping lvalues on target construct= ") talks about code like the following map(*p =3D 10) map(x =3D 20) map(x ? y[0] : p[1]) map(f(y)) is valid or not. The sentiment was to require that a 'map' clause list item must have a base pointer or a base variable. However, it looks as your examples would be valid in this regard. Can you f= ile a PR about this one? Referencing both to this testcase and to the OpenMP is= sue? (I do note that Clang and GCC reject the lvalue examples from the OpenMP is= sue but not your reference examples; those are accepted by clang++-14.) 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