From patchwork Mon Feb 19 12:45:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 85966 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 D7D423864C56 for ; Mon, 19 Feb 2024 12:45:37 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by sourceware.org (Postfix) with ESMTPS id D2813386187E for ; Mon, 19 Feb 2024 12:45:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D2813386187E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=baylibre.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D2813386187E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::432 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708346709; cv=none; b=PQFiTWXrWn4DZKfi567dGJveZU4k/g3WBtbtmFadltYGNptpzNJE32bvGt2RXFUPzAZiy5/OjBEtdtbZ2y0gzcEpfhJVP/SEtYTz2K0JPbfe9qdyiC3A+R3qZcKS0ZMvGLvVEz4SBOGyYcRJU3XEDLBI/rHBZUGZ7j4sOI6nS9w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708346709; c=relaxed/simple; bh=Y+VNjg3OZdsb9lYfGCrwMBpw9/5qwUAPNhnO/hhRtYg=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=RRcSmRnkCHJ3PnWCMHdxvse4BMSYyLUysC1z4C5+Fw+mNMS5FS6mXpA1gCGUzLvmRN64G876VbkLCIcDJM677gb/z/3/aVUw/tVfpDHSOAYTacbS/H31+vY9OLDXJ3czNUj8HqtdWE+XrsMesjnvlTgJCXe0VJxmRj/k6Opoq3M= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-x432.google.com with SMTP id ffacd0b85a97d-33d28468666so1197963f8f.0 for ; Mon, 19 Feb 2024 04:45:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1708346704; x=1708951504; darn=gcc.gnu.org; h=subject:from:to:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=cgrFik2nN2xMBKzrifwAVKn226cnk2oSDLHj8WMRFNA=; b=jRbGNU1QgKcsMhOud0uGsJW4KaDY+6q+J/oliECSK8sXszbf2Xx3vROnSaDF0qWKX/ UsKhLXzIkRTbCBnkK1Ugt2iYRO8szUgGPvyfM+0po+2TZUsmuj7zjakNlYl4Qov0wLJu QitTzNHVSrZcwEbJQKMXGegpQsDo4hcl0pLm9zHgpn//t8kVs8D5QENqaHAglzbsrp9k +brU6qD5iQ69mV9Th4gZ9gJjN28ahdf5CcWJ2I8O3GR+4IR5KVzFfYSX/TIVxzoJiqPU eazw8oETAr4oa4gciD0JQQ4n8iETlwVuqZ61xvyFwbgqjCPQ9lk3yRRvxoYdO/UGi0M8 rRrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708346704; x=1708951504; h=subject:from:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cgrFik2nN2xMBKzrifwAVKn226cnk2oSDLHj8WMRFNA=; b=t0MSJpYhMIiWe+nRrnKfDFSNw1T1qwfmfg+8W5f7g9dRUrs8cdiEauoY/02YSV4PLz lEGQIXDUG9T+jEG04Kc1CjTpQV0L5KOIic59oGaxh5j0qc5hWAihfzLd5WKvOaR58Zci J8o8HztcQ+sseyvsczcpwne85FYZ2Pe3bLMxFrHhX1RKCexHfUQ4ihkR7SGA061WXeZG j5vnjo+ivk5iwlPXnpdScQUOMMYSVoj76J62eSSVMbwjOnE4muwQbEeugNQ9Zjk/ykzG u1A5Dldnk3yqasOPojLSaQPua536V6cMtPRHSKeYnNoTwm1gETmWQF7kFBB43+SPQlU5 kvaw== X-Gm-Message-State: AOJu0YzH8JwW5A9WHlT5m1pDxk/I7wzTRDN1B5a0Kp9l7U6n4QRDIPfB yZXN5A0kXt/z5oAAZgCEvVf1Vg71/DTKaY1woICg3i9SbK13Um1xrTT0958EZKP0IppQF7V3zfD N X-Google-Smtp-Source: AGHT+IG2/kQSYayVg3brTRhKCmouDI41eZSGW/IMPPjJo6kQYRIXaUOUtwTihaIJD9svy6AKlPm7Hw== X-Received: by 2002:a5d:58d4:0:b0:33d:6156:e6e3 with SMTP id o20-20020a5d58d4000000b0033d6156e6e3mr382399wrf.16.1708346704320; Mon, 19 Feb 2024 04:45:04 -0800 (PST) Received: from ?IPV6:2001:16b8:2a3e:a000:b4dd:aa50:647b:4436? (200116b82a3ea000b4ddaa50647b4436.dip.versatel-1u1.de. [2001:16b8:2a3e:a000:b4dd:aa50:647b:4436]) by smtp.gmail.com with ESMTPSA id n9-20020a056000170900b0033d5fab6781sm1079784wrc.96.2024.02.19.04.45.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 19 Feb 2024 04:45:03 -0800 (PST) Message-ID: Date: Mon, 19 Feb 2024 13:45:03 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: gcc-patches From: Tobias Burnus Subject: [Patch] libgomp: Device load_image - minor num-funcs/vars check improvement X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 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 When debugging a linker issue, leading to a mismatch in the number of host/device functions, I was surprised by seeing one additional entry. Well, it turned out to be due to the ICV variable. This patch makes it more consistent. The "+1" is returned since r12-2769-g0bac793ed6bad2 (for the on-device omp_get_device_num), extended in r13-2545-g9f2fca56593a2b for a struct to support more ICV variables on the devices [to handle OMP_..._DEV environment variables]. As the value is returned unconditionally, it makes sense to use it both for the expected-value diagnostic and for the condition further below. Comments, suggestions, remarks? Tobias PS: Alternative would be to make the plugin's value depend on whether the data was loaded. But that would make the number-of-entries assert weaker and might cause corner-case issues when a slightly older libgomp plugin is used with the updated libgomp.so. Thus, I have settled for the attached variant. libgomp: Device load_image - improve minor num-funcs/vars check The run time library loads the offload functions and variable and optionally the ICV variable and returns the number of loaded items, which has to match the host side. The plugin returns "+1" (since GCC 12) for the ICV variable entry, independently whether it was loaded or not, but the var's value (start == end == 0) can be used to detect when this failed. Thus, we can tighten the assert check - which this commit does together with making the output less surprising - and simplify the condition further below. libgomp/ChangeLog: * plugin/plugin-gcn.c (GOMP_OFFLOAD_load_image): If ICV variable is is not available, decrement other_count and thus the return value. * plugin/plugin-nvptx.c (GOMP_OFFLOAD_load_image): Likewise. * target.c (gomp_load_image_to_device): Extend fatal-error message; simplify a condition. libgomp/target.c | 78 +++++++++++++++++++++++++------------------------------- 1 file changed, 35 insertions(+), 43 deletions(-) diff --git a/libgomp/target.c b/libgomp/target.c index 1367e9cce6c..456a9147154 100644 --- a/libgomp/target.c +++ b/libgomp/target.c @@ -2355,15 +2355,14 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version, num_ind_funcs ? (uint64_t *) host_ind_func_table : NULL); - if (num_target_entries != num_funcs + num_vars - /* "+1" due to the additional ICV struct. */ - && num_target_entries != num_funcs + num_vars + 1) + /* The "+1" is due to the additional ICV struct. */ + if (num_target_entries != num_funcs + num_vars + 1) { gomp_mutex_unlock (&devicep->lock); if (is_register_lock) gomp_mutex_unlock (®ister_lock); gomp_fatal ("Cannot map target functions or variables" - " (expected %u, have %u)", num_funcs + num_vars, + " (expected %u + %u + 1, have %u)", num_funcs, num_vars, num_target_entries); } @@ -2447,48 +2446,41 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version, array++; } - /* Last entry is for a ICVs variable. - Tolerate case where plugin does not return those entries. */ - if (num_funcs + num_vars < num_target_entries) + /* Last entry is for the ICV struct variable; if absent, start = end = 0. */ + struct addr_pair *icv_var = &target_table[num_funcs + num_vars]; + if (icv_var->start != 0) { - struct addr_pair *var = &target_table[num_funcs + num_vars]; - - /* Start address will be non-zero for the ICVs variable if - the variable was found in this image. */ - if (var->start != 0) + /* The index of the devicep within devices[] is regarded as its + 'device number', which is different from the per-device type + devicep->target_id. */ + int dev_num = (int) (devicep - &devices[0]); + struct gomp_offload_icvs *icvs = get_gomp_offload_icvs (dev_num); + size_t var_size = icv_var->end - icv_var->start; + if (var_size != sizeof (struct gomp_offload_icvs)) { - /* The index of the devicep within devices[] is regarded as its - 'device number', which is different from the per-device type - devicep->target_id. */ - int dev_num = (int) (devicep - &devices[0]); - struct gomp_offload_icvs *icvs = get_gomp_offload_icvs (dev_num); - size_t var_size = var->end - var->start; - if (var_size != sizeof (struct gomp_offload_icvs)) - { - gomp_mutex_unlock (&devicep->lock); - if (is_register_lock) - gomp_mutex_unlock (®ister_lock); - gomp_fatal ("offload plugin managed 'icv struct' not of expected " - "format"); - } - /* Copy the ICVs variable to place on device memory, hereby - actually designating its device number into effect. */ - gomp_copy_host2dev (devicep, NULL, (void *) var->start, icvs, - var_size, false, NULL); - splay_tree_key k = &array->key; - k->host_start = (uintptr_t) icvs; - k->host_end = - k->host_start + (size_mask & sizeof (struct gomp_offload_icvs)); - k->tgt = tgt; - k->tgt_offset = var->start; - k->refcount = REFCOUNT_INFINITY; - k->dynamic_refcount = 0; - k->aux = NULL; - array->left = NULL; - array->right = NULL; - splay_tree_insert (&devicep->mem_map, array); - array++; + gomp_mutex_unlock (&devicep->lock); + if (is_register_lock) + gomp_mutex_unlock (®ister_lock); + gomp_fatal ("offload plugin managed 'icv struct' not of expected " + "format"); } + /* Copy the ICVs variable to place on device memory, hereby + actually designating its device number into effect. */ + gomp_copy_host2dev (devicep, NULL, (void *) icv_var->start, icvs, + var_size, false, NULL); + splay_tree_key k = &array->key; + k->host_start = (uintptr_t) icvs; + k->host_end = + k->host_start + (size_mask & sizeof (struct gomp_offload_icvs)); + k->tgt = tgt; + k->tgt_offset = icv_var->start; + k->refcount = REFCOUNT_INFINITY; + k->dynamic_refcount = 0; + k->aux = NULL; + array->left = NULL; + array->right = NULL; + splay_tree_insert (&devicep->mem_map, array); + array++; } free (target_table);