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 12FCF387085A for ; Fri, 26 Jun 2020 09:21:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 12FCF387085A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=Thomas_Schwinge@mentor.com IronPort-SDR: waGFi3roYchoXd8TpWdERwgSW335uquDkfIeMDfV6D1api/ELHbHipt+W4nB7Rxv/SxVnGXZbn u4vVX0dsKUf2+5JHqug4KgK1knubCJJt6+MIcFT85Ve8EvOuXIug8hhrOfSkyNzejW8pI6F7fB 0Zm7stuCSyypKum4pm+SpaRBY64WgiXpCH7So15176FWYHH2EXgVEHdeEhaN3h4QTOZDqt6paM f5x8Rpd87vYaqSowT3L44LRXyoF6r5fLkGbtuzUZYktWVIF7LCEA11b6Gm2jrtSTkRf7nYhR2Z 2QM= X-IronPort-AV: E=Sophos;i="5.75,283,1589270400"; d="scan'208";a="50342317" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 26 Jun 2020 01:21:02 -0800 IronPort-SDR: UQACEtL6oI60SylLcqcJcfWWSSIv8cEfNQI1D2hxHXCt9rE5imLNnIIODoSrduqpHlnKauEmxW 6Du9S2N/n3Ln3EJJqfe5Rz1shp5uL8cCmL/5333mGOvyZ/r8HFKIBgAQ/pvxCIB6BkVYcOObBF OEuTXjjaoiOhR6Fd7Qw8+Q4pqg4i159sZwMTmLGO8NlR7lCUIPZG+q/nOXtf3XKEVnedb4+aQq Zx4GjMbiCs0q3h1bm4vNCXybPQuZM+8NyJ+YdgEnzcrOiLtRsOgZIa4BgVFTCeCDxzJyEw2WaL E8c= From: Thomas Schwinge To: Julian Brown CC: , Jakub Jelinek , Tobias Burnus , Subject: Re: [PATCH 07/13] OpenACC 2.6 deep copy: libgomp parts In-Reply-To: <65540b92dff74db1f15af930f87f7096d03e7efe.1576648001.git.julian@codesourcery.com> References: <65540b92dff74db1f15af930f87f7096d03e7efe.1576648001.git.julian@codesourcery.com> User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Fri, 26 Jun 2020 11:20:40 +0200 Message-ID: <87366i18uv.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Jun 2020 09:21:07 -0000 Hi Julian! On 2019-12-17T22:03:47-0800, Julian Brown wrote: > This part contains the libgomp runtime support for the GOMP_MAP_ATTACH an= d > GOMP_MAP_DETACH mapping kinds (etc.), as introduced by the front-end > patches following in this series. > --- a/libgomp/target.c > +++ b/libgomp/target.c > @@ -1534,6 +1571,18 @@ gomp_unmap_vars_internal (struct target_mem_desc *= tgt, bool do_copyfrom, This is the code path at the end of a structured OpenACC 'data' construct. > + /* We must perform detachments before any copies back to the host. */ > + for (i =3D 0; i < tgt->list_count; i++) > + { > + splay_tree_key k =3D tgt->list[i].key; > + > + if (k !=3D NULL && tgt->list[i].do_detach) > + gomp_detach_pointer (devicep, aq, k, tgt->list[i].key->host_start > + + tgt->list[i].offset, > + k->refcount =3D=3D 1, NULL); > + } Can you please explain (as a source code comment) the logic for here using 'k->refcount =3D=3D 1' for the 'bool finalize' parameter of 'gomp_detach_pointer'; this somehow feels "strange"? Nonwithstanding the question whether that's a valid thing to do or not, but doesn't the current code hide the "attach count underflow" error if you reach the above code with 'attach_count =3D=3D 0' (user already explicitly 'detach'ed), but then given 'k->refcount =3D=3D 1' (thus 'finalize' semantics), 'gomp_detach_pointer' will then re-initialize 'attach_count =3D 1', and then do another 'gomp_copy_host2dev', etc. instead of emitting an error. (I have not attempted to produce a libgomp test case.) Shouldn't this just always be 'finalize =3D false' given that there is no 'finalize' semantics for 'detach' on a structured OpenACC 'data' constructs -- at least that's what I remember right now? Gr=C3=BC=C3=9Fe Thomas ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstra=C3=9Fe 201, 80634 M=C3=BCnch= en / Germany Registergericht M=C3=BCnchen HRB 106955, Gesch=C3=A4ftsf=C3=BChrer: Thomas = Heurung, Alexander Walter