From patchwork Fri Jan 19 20:07:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 1888653 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.a=rsa-sha256 header.s=20230601 header.b=b+ToojRG; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TGrK51q6Bz1yQ0 for ; Sat, 20 Jan 2024 07:08:25 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 275903858C3A for ; Fri, 19 Jan 2024 20:08:23 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com [IPv6:2a00:1450:4864:20::22f]) by sourceware.org (Postfix) with ESMTPS id 633D13858C2A for ; Fri, 19 Jan 2024 20:08:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 633D13858C2A 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 633D13858C2A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::22f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705694883; cv=none; b=owpB4rQxWUNg5bNyn+KMKwjMkCgaOx/zUtB1ua3USYQwgqOLVNSuB7q1rbqNEmXDgSUg5riksfaCV5htdZYV1i6ihDSldmrUbTvSvvxG4KzA+GnEcg/3BihTu9j0t85DTWSYcBDap7uchkO30E6oXndhNAfUPHKEMWHylmKoiMw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705694883; c=relaxed/simple; bh=syfyU33Nka069D1gG0rknlMU3/8PEYfdM7RWAniT3QQ=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=ZQLY9Zn/ubFWu5wR4jJYwdlgpMYVWEGwYvDwCRdYVJ2OkvebIfLSA0y1TlQWux4I+p4Ll2nZjVQiE+5IP67iYOXMKzvMJEWvv3yyO0UVTDWXeD8PkaSh05BTQmnug4s652JIujB2VX4osU0GwqAskIRbU8LnPVvatD+1Om1tq78= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lj1-x22f.google.com with SMTP id 38308e7fff4ca-2cceb5f0918so13190371fa.2 for ; Fri, 19 Jan 2024 12:08:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1705694878; x=1706299678; 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=C15ooJ3VNuyTpeiRCEoKXpgKFJYw17VR/LHv/lgCFWI=; b=b+ToojRGLpgATzwKxiSamHZOvmQnLjPlNzY0H4iN+EXP9NBoxkMgtVyB1rRT/TearC aqUmq02EsQ4cseZVrVJKXTeOjy5AVQ/dmAk5jSxOmcbiH9snyaoExWqVY87XHceaKrPt 1dVXzFjZmoqRVQUrSqcktlrYVcAPEqr1/H9b2AXod6EjEJ6gik2TksFZ/TXW0dAN0SPL ZZBzwbLzsIi54sfBSDdqsd6im1v6DOI7XXjHrqK1oGiFIxOtrhsS1C7XY1yYT9NmmgKR g1xnWr/8xMz8oGYsB928e5Vblotz3wXA+hbmr1pwUFwjqdNuAK1pw6kXptXdgHcyijpF roqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705694878; x=1706299678; 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=C15ooJ3VNuyTpeiRCEoKXpgKFJYw17VR/LHv/lgCFWI=; b=KtXUXHWqybQG95lrjg1a124yoFqowxwEEyXOfHy77LoNyoQmds2CkD3fKf1Gp3Ew6E WHc3PpybIcfdxiijfV8YDyu7/ZL4jQoRSo7hQITMtJ/AMtsgZtRjZB69FKZp3N2sg2Fy YKDq0YCM06HV3IpYUN4O3twUWrvw2MLmaVKsDwLPTo40ru+p4qgIWgPHpxb2Il5JJFb2 hPfnBGibJYa2CevL82wfHRek3iLnLiEMHwcXbJmoLhq8aOwvt+9sxjl61qnw9q6E+u48 bvXH7lPjWTDkgysSiSJ4A50Z89wYCXbgxYKAwi+AJgOzQ+BAleoEekyFQ54stpey0il5 u06A== X-Gm-Message-State: AOJu0Yz6mtE9udcS8UYy8GkCN6+ZDbPXVQKWb3EKyJnN5iyVqQ34hgo2 W9kPB/uaUK0+TALXmn4WXt5g23f1E6QEvu5YgLhJV1hU4fsGUGNqjEnUV4GiZ857Df+CfFlTICr ttT1mgw== X-Google-Smtp-Source: AGHT+IHtoUI08D3i8pou4i8TknMr5XHmcev1If3YBYiu6sidVfAajk3CzHjhUqTdAuP+kmnvCJ8xiQ== X-Received: by 2002:a05:6512:130c:b0:50e:7cbf:8213 with SMTP id x12-20020a056512130c00b0050e7cbf8213mr67830lfu.127.1705694878350; Fri, 19 Jan 2024 12:07:58 -0800 (PST) Received: from ?IPV6:2001:16b8:3fec:9600:be03:58ff:fe31:f74? ([2001:16b8:3fec:9600:be03:58ff:fe31:f74]) by smtp.gmail.com with ESMTPSA id gz10-20020a170906f2ca00b00a2ed743ec47sm3535953ejb.121.2024.01.19.12.07.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Jan 2024 12:07:57 -0800 (PST) Message-ID: <3a1d7856-47fd-4e00-8b08-1e985cac6130@baylibre.com> Date: Fri, 19 Jan 2024 21:07:56 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: gcc-patches , Andrew Stubbs From: Tobias Burnus Subject: [patch][gcn] mkoffload: Fix linking with "-g"; fix file deletion; improve diagnostic [PR111966] X-Spam-Status: No, score=-13.7 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+incoming=patchwork.ozlabs.org@gcc.gnu.org This patch fixes PR111966, i.e. when compiling offloaded code with "-g" but without "-march=", mkoffload created a file with e_flags set to gfx803/fiji as architecture - while all other files used gfx900, which the linker did not like. Reason: When the default was changed, this flag was missed. When passing -march=... instead of relying on the default, it works. Additionally, it fixed a bug with dangling pointers and multiple deletion attempts for the same file, leading normally only to the accumulation of /tmp/cc*.mkoffload.dbg.o files. And, finally,  when building with a recent GCC; it warned about missing %<...%> or %qs quotes. I added a couple to reduce the number of warnings. OK for mainline? — I think the /tmp/cc*.mkoffload.dbg.o part of the patch could also be backported to GCC 13 (and 12) if deemed to be useful. Tobias [gcn] mkoffload: Fix linking with "-g"; fix file deletion; improve diagnostic [PR111966] With debugging enabled, '*.mkoffload.dbg.o' files are generated. The e_flags header of all *.o files must be the same - otherwise, the linker complains. Since r14-4734-g56ed1055b2f40ac162ae8d382280ac07a33f789f the -march= default is now gfx900. If compiling without any -march= flag, the default value is used by the compiler but not passed to mkoffload. Hence, mkoffload.cc's uses its own default for march - unfortunately, it still had gfx803/fiji as default, leading to the linker error: 'incompatible mach'. Solution: Update the default to gfx900. While debugging it, I saw that /tmp/cc*.mkoffload.dbg.o kept accumulating; there were a couple of issues with the handling: * dbgobj was always added to files_to_cleanup * If copy_early_debug_info returned true, dbgobj was added again -> pointless and in theory a race if the same file was added in the faction of a second. * If copy_early_debug_info returned false, - In exactly one case, it already deleted the file it self (same potential race as above) - The pointer dbgobj was freed - such that files_to_cleanup contained a dangling pointer - probably the reason that stale files remained. Solution: Only if copy_early_debug_info returns true, dbgobj is added to files_to_cleanup. If it returns false, the file is unlinked before freeing the pointer. When compiling, GCC warned about several fatal_error messages as having no %<...%> or %qs quotes. This patch now silences several of those warnings by using those quotes. gcc/ChangeLog: PR other/111966 * config/gcn/mkoffload.cc (elf_arch): Change default to gfx900 to match the compiler default. (simple_object_copy_lto_debug_sections): Never unlink the outfile on error as the caller does so. (maybe_unlink, compile_native): Use %<...%> and %qs in fatal_error. (main): Likewise. Fix 'mkoffload.dbg.o' cleanup. Signed-off-by: Tobias Burnus gcc/config/gcn/mkoffload.cc | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc index d4cd509089e..0d0e7bac9b2 100644 --- a/gcc/config/gcn/mkoffload.cc +++ b/gcc/config/gcn/mkoffload.cc @@ -124,7 +124,7 @@ static const char *gcn_dumpbase; static struct obstack files_to_cleanup; enum offload_abi offload_abi = OFFLOAD_ABI_UNSET; -uint32_t elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX803; // Default GPU architecture. +uint32_t elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX900; // Default GPU architecture. uint32_t elf_flags = EF_AMDGPU_FEATURE_SRAMECC_ANY_V4; static int gcn_stack_size = 0; /* Zero means use default. */ @@ -154,7 +154,7 @@ maybe_unlink (const char *file) if (!save_temps) { if (unlink_if_ordinary (file) && errno != ENOENT) - fatal_error (input_location, "deleting file %s: %m", file); + fatal_error (input_location, "deleting file %qs: %m", file); } else if (verbose) fprintf (stderr, "[Leaving %s]\n", file); @@ -320,10 +320,7 @@ copy_early_debug_info (const char *infile, const char *outfile) errmsg = simple_object_copy_lto_debug_sections (inobj, outfile, &err, true); if (errmsg) - { - unlink_if_ordinary (outfile); - return false; - } + return false; simple_object_release_read (inobj); close (infd); @@ -804,7 +801,7 @@ compile_native (const char *infile, const char *outfile, const char *compiler, const char *collect_gcc_options = getenv ("COLLECT_GCC_OPTIONS"); if (!collect_gcc_options) fatal_error (input_location, - "environment variable COLLECT_GCC_OPTIONS must be set"); + "environment variable % must be set"); struct obstack argv_obstack; obstack_init (&argv_obstack); @@ -859,11 +856,11 @@ main (int argc, char **argv) obstack_init (&files_to_cleanup); if (atexit (mkoffload_cleanup) != 0) - fatal_error (input_location, "atexit failed"); + fatal_error (input_location, "% failed"); char *collect_gcc = getenv ("COLLECT_GCC"); if (collect_gcc == NULL) - fatal_error (input_location, "COLLECT_GCC must be set."); + fatal_error (input_location, "% must be set"); const char *gcc_path = dirname (ASTRDUP (collect_gcc)); const char *gcc_exec = basename (ASTRDUP (collect_gcc)); @@ -909,7 +906,7 @@ main (int argc, char **argv) if (!found) fatal_error (input_location, - "offload compiler %s not found", GCC_INSTALL_NAME); + "offload compiler %qs not found", GCC_INSTALL_NAME); /* We may be called with all the arguments stored in some file and passed with @file. Expand them into argv before processing. */ @@ -931,7 +928,7 @@ main (int argc, char **argv) offload_abi = OFFLOAD_ABI_ILP32; else fatal_error (input_location, - "unrecognizable argument of option " STR); + "unrecognizable argument of option %<" STR "%>"); } #undef STR else if (strcmp (argv[i], "-fopenmp") == 0) @@ -994,7 +991,8 @@ main (int argc, char **argv) } if (!(fopenacc ^ fopenmp)) - fatal_error (input_location, "either -fopenacc or -fopenmp must be set"); + fatal_error (input_location, + "either %<-fopenacc%> or %<-fopenmp%> must be set"); const char *abi; switch (offload_abi) @@ -1066,7 +1064,7 @@ main (int argc, char **argv) cfile = fopen (gcn_cfile_name, "w"); if (!cfile) - fatal_error (input_location, "cannot open '%s'", gcn_cfile_name); + fatal_error (input_location, "cannot open %qs", gcn_cfile_name); /* Currently, we only support offloading in 64-bit configurations. */ if (offload_abi == OFFLOAD_ABI_LP64) @@ -1130,7 +1128,6 @@ main (int argc, char **argv) } else dbgobj = make_temp_file (".mkoffload.dbg.o"); - obstack_ptr_grow (&files_to_cleanup, dbgobj); /* If the copy fails then just ignore it. */ if (copy_early_debug_info (argv[ix], dbgobj)) @@ -1139,7 +1136,10 @@ main (int argc, char **argv) obstack_ptr_grow (&files_to_cleanup, dbgobj); } else - free (dbgobj); + { + maybe_unlink (dbgobj); + free (dbgobj); + } } } } @@ -1214,7 +1214,7 @@ main (int argc, char **argv) out = fopen (gcn_s2_name, "w"); if (!out) - fatal_error (input_location, "cannot open '%s'", gcn_s2_name); + fatal_error (input_location, "cannot open %qs", gcn_s2_name); process_asm (in, out, cfile);