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 A60B33851C0D for ; Thu, 25 Jun 2020 11:04:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A60B33851C0D 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: uHgiwQGEU5yBOTdU9V7XEASsnNFQIWmwr6+cHNZ6egkyGLgweTBurxSZnUx/EsmmbatJwbqGHt itbu5T6yD7IsW7Gki7hL+ZT3TyaqqR7IsB1i2+CP30HkRnKTQjBInTdXqhMczL372YkOYexQsW 2SsceCjGPlz09mrSR27+e11Nt9v6UPRgauLUai+GhLbdKsulqa1UMFhxKufPzWZF2g04/rkRPR GBdocwEXUJeofR55dHEuCX0omz97vCP+mxwaftSelfRwW5qx0ocL7TjPl7B/yYHDNZEZKUhfUb r0E= X-IronPort-AV: E=Sophos;i="5.75,279,1589270400"; d="scan'208";a="52320963" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 25 Jun 2020 03:04:13 -0800 IronPort-SDR: IJiq0IIKGJAO6Q222LSHkh4f/eiREoHmFVirZNiqsDu6PoCv4o/ENIfpiXFr5ZxiR0/au6ypwy kJbDblroGORGCl8BBCbvz5vk/Ib28k/XGebbaMqhjmtwtGWfxYPwlJuNIV7omelROx+Q3l+ZBU Nl4V1CzpUZ3/V9osytKO2Pfmw9dOK+2SjqpsipybQhZy75RoQLBjZu6V6xLTxoNGe3MptH6vzI tra7YBq55J1it4Ahoq7jpbjPy/JCUGav87FpwTrPt2lZ5xtnihg/9CMoazl0qS/cKA2oAG6Ncn aoI= From: Thomas Schwinge To: Julian Brown CC: , Jakub Jelinek , Subject: Re: [PATCH 02/13] OpenACC reference count overhaul In-Reply-To: <87zha39asn.fsf@euler.schwinge.homeip.net> References: <491e3ca360313930f8f2f5686ffd386cf2fad04e.1576648001.git.julian@codesourcery.com> <87zha39asn.fsf@euler.schwinge.homeip.net> User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Thu, 25 Jun 2020 13:03:53 +0200 Message-ID: <87eeq31k6e.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.9 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: Thu, 25 Jun 2020 11:04:17 -0000 Hi Julian! Ping, in particular my question about different 'GOMP_MAP_FORCE_FROM' vs. 'GOMP_MAP_FROM' handling. (I have not yet looked whether 'GOMP_MAP_ALWAYS_FROM' may be generate nowadays, given your pending front end/middle end patches.) On 2020-05-19T17:58:16+0200, I wrote: > On 2019-12-17T22:02:27-0800, Julian Brown wrote= : >> --- a/libgomp/oacc-mem.c >> +++ b/libgomp/oacc-mem.c > > (Unhelpful diff trimmed.) > >> +/* Unmap variables for OpenACC "exit data", with optional finalization >> + (affecting all mappings in this operation). */ > >> +static void >> +goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t map= num, >> + void **hostaddrs, size_t *sizes, >> + unsigned short *kinds, bool finalize, goacc_aq aq= ) >> +{ >> + gomp_mutex_lock (&acc_dev->lock); > >> + for (size_t i =3D 0; i < mapnum; ++i) >> { > >> + unsigned char kind =3D kinds[i] & 0xff; >> + bool copyfrom =3D false; > >> + switch (kind) > >> + case GOMP_MAP_FROM: >> + case GOMP_MAP_FORCE_FROM: >> + case GOMP_MAP_ALWAYS_FROM: >> + copyfrom =3D true; >> + /* Fallthrough. */ > > What is the case that a 'GOMP_MAP_ALWAYS_FROM' would be generated for > OpenACC code? Putting an 'assert' here, it never triggers, given the > current set of libgomp test cases. If there is such a case, we should > add a test case, otherwise, I suggest we do put an 'assert' here (whilst > leaving in the supposedly correct code, if you'd like), to document that > this not currently expected, and thus not tested? > >> + >> + case GOMP_MAP_TO_PSET: >> + case GOMP_MAP_POINTER: >> + case GOMP_MAP_DELETE: >> + case GOMP_MAP_RELEASE: >> + { >> + struct splay_tree_key_s cur_node; >> + cur_node.host_start =3D (uintptr_t) hostaddrs[i]; >> + cur_node.host_end =3D cur_node.host_start >> + + (kind =3D=3D GOMP_MAP_POINTER >> + ? sizeof (void *) : sizes[i]); >> + splay_tree_key n >> + =3D splay_tree_lookup (&acc_dev->mem_map, &cur_node); >> + >> + if (n =3D=3D NULL) >> + continue; >> + >> + if (finalize) >> + { >> + if (n->refcount !=3D REFCOUNT_INFINITY) >> + n->refcount -=3D n->virtual_refcount; >> + n->virtual_refcount =3D 0; >> + } >> + >> + if (n->virtual_refcount > 0) >> + { >> + if (n->refcount !=3D REFCOUNT_INFINITY) >> + n->refcount--; >> + n->virtual_refcount--; >> + } >> + else if (n->refcount > 0 && n->refcount !=3D REFCOUNT_INFINITY) >> + n->refcount--; >> + >> + if (copyfrom >> + && (kind !=3D GOMP_MAP_FROM || n->refcount =3D=3D 0)) >> + gomp_copy_dev2host (acc_dev, aq, (void *) cur_node.host_start= , >> + (void *) (n->tgt->tgt_start + n->tgt_offs= et >> + + cur_node.host_start >> + - n->host_start), >> + cur_node.host_end - cur_node.host_start); > > That 'kind !=3D GOMP_MAP_FROM' conditional looks wrong to me. This shoul= d > instead be 'kind =3D=3D GOMP_MAP_ALWAYS_FROM'? Or, get removed, together > with the 'GOMP_MAP_ALWAYS_FROM' handling above? But definitely > 'GOMP_MAP_FORCE_FROM' and 'GOMP_MAP_FROM' need to be handled the same, as > far as I can tell? > >> + >> + if (n->refcount =3D=3D 0) >> + gomp_remove_var_async (acc_dev, n, aq); >> + } >> + break; >> + default: >> + gomp_fatal (">>>> goacc_exit_data_internal UNHANDLED kind 0x%.2x"= , >> + kind); >> } >> } >> >> gomp_mutex_unlock (&acc_dev->lock); > >> } 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