From patchwork Fri Mar 24 15:36:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 66860 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3FCD73882675 for ; Fri, 24 Mar 2023 15:36:53 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 31BF538515C9 for ; Fri, 24 Mar 2023 15:36:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 31BF538515C9 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.98,288,1673942400"; d="scan'208,223";a="325553" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 24 Mar 2023 07:36:34 -0800 IronPort-SDR: hcnpRwhseOUHqbUBGq5V8VtXg1xHvE1XWUZrMW7RGo/qgurnMSYf1L0F56XdNBbIa5IOmDjXyG Sh2UrUs/oh5Dqg9Tb2UMp7AnijyP7ZwRV2Q5DRuq478ISTGOW6xes8gDJZLIKrqzXpHxUYux6c s5XR+lD+DzuvNtoQoNCiIxkIDOzZC+Ac7m2Kq9vKlczrjlBFzX3orELrwejzJ+JbJKovx6JsH5 +kRJGVxdOyd5ET70jIdEDsXc3t1u2EXm+FSAFkxz68f7UFR1U6mf/PQItL3sA7a+HnwY1PjTwr NL0= From: Thomas Schwinge To: CC: Chung-Lin Tang , Jakub Jelinek , Tobias Burnus Subject: [og12] In 'libgomp/target.c:gomp_unmap_vars_internal', defer 'gomp_remove_var' (was: [PATCH, v2, OpenMP 5.0, libgomp] Structure element mapping for OpenMP 5.0) References: <12b667d2-09fe-0640-2622-c78ab0b52f87@codesourcery.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Fri, 24 Mar 2023 16:36:28 +0100 Message-ID: <87ileq9n5v.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, 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 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: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi! On 2020-12-04T22:15:46+0800, Chung-Lin Tang wrote: > this is a new version of the structure element mapping patch for OpenMP 5.0 requirement > changes. > (gomp_exit_data): [...] > adjust to queue splay-tree keys for removal > after main loop. > --- a/libgomp/target.c > +++ b/libgomp/target.c > @@ -2485,14 +2714,17 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum, > + int nrmvars = 0; > + splay_tree_key remove_vars[mapnum]; > + > for (i = 0; i < mapnum; i++) > { > - if (k->refcount == 0) > - gomp_remove_var (devicep, k); > + > + /* Structure elements lists are removed altogether at once, which > + may cause immediate deallocation of the target_mem_desc, causing > + errors if we still have following element siblings to copy back. > + While we're at it, it also seems more disciplined to simply > + queue all removals together for processing below. > + > + Structured block unmapping (i.e. gomp_unmap_vars_internal) should > + not have this problem, since they maintain an additional > + tgt->refcount = 1 reference to the target_mem_desc to start with. > + */ > + if (do_remove) > + remove_vars[nrmvars++] = k; > } > > + for (int i = 0; i < nrmvars; i++) > + gomp_remove_var (devicep, remove_vars[i]); > + > gomp_mutex_unlock (&devicep->lock); > } Upcoming work of mine actually now does require this change also for 'gomp_unmap_vars_internal', such that 'gomp_remove_var' be deferred until after all 'gomp_copy_dev2host' calls have been handled. I've pushed to devel/omp/gcc-12 commit 65037818987ffce7d6f466fa8bde13e9f59a3218 "In 'libgomp/target.c:gomp_unmap_vars_internal', defer 'gomp_remove_var'", see attached. Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 From 65037818987ffce7d6f466fa8bde13e9f59a3218 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Tue, 14 Mar 2023 19:42:12 +0100 Subject: [PATCH] In 'libgomp/target.c:gomp_unmap_vars_internal', defer 'gomp_remove_var' An upcoming change requires that 'gomp_remove_var' be deferred until after all 'gomp_copy_dev2host' calls have been handled. Do this likewise to how commit 275c736e732d29934e4d22e8f030d5aae8c12a52 "libgomp: Structure element mapping for OpenMP 5.0" changed 'gomp_exit_data'. libgomp/ * target.c (gomp_unmap_vars_internal): Queue splay-tree keys for removal after main loop. --- libgomp/ChangeLog.omp | 3 +++ libgomp/target.c | 34 +++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp index 85ebab14ba8..9360db66b03 100644 --- a/libgomp/ChangeLog.omp +++ b/libgomp/ChangeLog.omp @@ -1,5 +1,8 @@ 2023-03-24 Thomas Schwinge + * target.c (gomp_unmap_vars_internal): Queue splay-tree keys for + removal after main loop. + PR other/76739 * oacc-parallel.c (GOACC_parallel_keyed): Given OpenACC 'async', defer 'free' of non-contiguous array support data structures. diff --git a/libgomp/target.c b/libgomp/target.c index aaa597f6610..107c3567a30 100644 --- a/libgomp/target.c +++ b/libgomp/target.c @@ -2180,6 +2180,9 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom, false, NULL); } + size_t nrmvars = 0; + splay_tree_key remove_vars[tgt->list_count]; + for (i = 0; i < tgt->list_count; i++) { splay_tree_key k = tgt->list[i].key; @@ -2201,16 +2204,21 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom, (void *) (k->tgt->tgt_start + k->tgt_offset + tgt->list[i].offset), tgt->list[i].length); + /* Queue all removals together for processing below. + See also 'gomp_exit_data'. */ if (do_remove) - { - struct target_mem_desc *k_tgt = k->tgt; - bool is_tgt_unmapped = gomp_remove_var (devicep, k); - /* It would be bad if TGT got unmapped while we're still iterating - over its LIST_COUNT, and also expect to use it in the following - code. */ - assert (!is_tgt_unmapped - || k_tgt != tgt); - } + remove_vars[nrmvars++] = k; + } + + for (i = 0; i < nrmvars; i++) + { + splay_tree_key k = remove_vars[i]; + struct target_mem_desc *k_tgt = k->tgt; + bool is_tgt_unmapped = gomp_remove_var (devicep, k); + /* It would be bad if TGT got unmapped while we're still iterating over + its LIST_COUNT, and also expect to use it in the following code. */ + assert (!is_tgt_unmapped + || k_tgt != tgt); } if (aq) @@ -4157,7 +4165,7 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum, false, NULL); } - int nrmvars = 0; + size_t nrmvars = 0; splay_tree_key remove_vars[mapnum]; for (i = 0; i < mapnum; i++) @@ -4220,10 +4228,6 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum, errors if we still have following element siblings to copy back. While we're at it, it also seems more disciplined to simply queue all removals together for processing below. - - Structured block unmapping (i.e. gomp_unmap_vars_internal) should - not have this problem, since they maintain an additional - tgt->refcount = 1 reference to the target_mem_desc to start with. */ if (do_remove) remove_vars[nrmvars++] = k; @@ -4238,7 +4242,7 @@ gomp_exit_data (struct gomp_device_descr *devicep, size_t mapnum, } } - for (int i = 0; i < nrmvars; i++) + for (i = 0; i < nrmvars; i++) gomp_remove_var (devicep, remove_vars[i]); gomp_mutex_unlock (&devicep->lock); -- 2.25.1