From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 1709F385041E for ; Wed, 12 Oct 2022 09:21:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1709F385041E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id D89DA21FA6; Wed, 12 Oct 2022 09:21:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1665566480; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XdL5Bxbh94se3EQcMfae0HUM+JRCgm5aLW5ez3ieI6E=; b=deMkpcImqJdXVZ6hT/fMauG6ZnpW8xo9WzLcDFxufI20yEgesh5KYNeNEYTiHm70KoQb7X fkqOCdD/C5r1hHJKgGIjimGXNuPzA3iCr/lxh7hrWZXNJrmaM3Q8YyIyDwJRy64ToMJxig e6SbjTwsWXeqSQXzp8n6OuimlF/hFkM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1665566480; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XdL5Bxbh94se3EQcMfae0HUM+JRCgm5aLW5ez3ieI6E=; b=+2woXfuknzFeEwvWLDFM/xHWdAZFBUGzvYOoV4wwEW123yLxkkkoyMz8VT7Uk1hvnolzra HdBE8+ruZCNWjqBw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 84F7B13A5C; Wed, 12 Oct 2022 09:21:20 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id b05AHhCHRmPFdQAAMHmgww (envelope-from ); Wed, 12 Oct 2022 09:21:20 +0000 Message-ID: <4893b6df-b624-9af7-fb8b-9ff0c5385d83@suse.cz> Date: Wed, 12 Oct 2022 11:21:19 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: Restore default 'sorry' 'TARGET_ASM_CONSTRUCTOR', 'TARGET_ASM_DESTRUCTOR' (was: [PATCH 1/3] STABS: remove -gstabs and -gxcoff functionality) Content-Language: en-US To: Thomas Schwinge , gcc-patches@gcc.gnu.org Cc: Richard Biener , Tom de Vries References: <10a94ccc-e01b-b98a-0fcb-cd661c10c315@suse.cz> <87fsfviww8.fsf@euler.schwinge.homeip.net> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= In-Reply-To: <87fsfviww8.fsf@euler.schwinge.homeip.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_NONE,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 List-Id: On 10/10/22 16:19, Thomas Schwinge wrote: > Hi! > > On 2022-09-01T12:05:23+0200, Martin Liška wrote: >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> I've also built all cross compilers. > > First: thanks for that: clean up plus "built all cross compilers"! > > But yet, I've now tracked down an issue related to these changes, > apparently only visible via the nvptx back end -- and quite > non-obvious... ;-) > >> --- a/gcc/config/nvptx/nvptx.cc >> +++ b/gcc/config/nvptx/nvptx.cc >> @@ -52,7 +52,6 @@ >> #include "tm-preds.h" >> #include "tm-constrs.h" >> #include "langhooks.h" >> -#include "dbxout.h" >> #include "cfgrtl.h" >> #include "gimple.h" >> #include "stor-layout.h" > >> --- a/gcc/dbxout.cc >> +++ /dev/null >> @@ -1,3936 +0,0 @@ >> -/* Output dbx-format symbol table information from GNU compiler. > > The "dbx-format symbol table information" stuff indeed is not relevant > anymore, but: > >> -/* Record an element in the table of global destructors. SYMBOL is >> - a SYMBOL_REF of the function to be called; PRIORITY is a number >> - between 0 and MAX_INIT_PRIORITY. */ >> - >> -void >> -default_stabs_asm_out_destructor (rtx symbol ATTRIBUTE_UNUSED, >> - int priority ATTRIBUTE_UNUSED) >> -{ >> -#if defined DBX_DEBUGGING_INFO || defined XCOFF_DEBUGGING_INFO >> - /* Tell GNU LD that this is part of the static destructor set. >> - This will work for any system that uses stabs, most usefully >> - aout systems. */ >> - dbxout_begin_simple_stabs ("___DTOR_LIST__", 22 /* N_SETT */); >> - dbxout_stab_value_label (XSTR (symbol, 0)); >> -#else >> - sorry ("global destructors not supported on this target"); >> -#endif >> -} >> - >> -/* Likewise for global constructors. */ >> - >> -void >> -default_stabs_asm_out_constructor (rtx symbol ATTRIBUTE_UNUSED, >> - int priority ATTRIBUTE_UNUSED) >> -{ >> -#if defined DBX_DEBUGGING_INFO || defined XCOFF_DEBUGGING_INFO >> - /* Tell GNU LD that this is part of the static destructor set. >> - This will work for any system that uses stabs, most usefully >> - aout systems. */ >> - dbxout_begin_simple_stabs ("___CTOR_LIST__", 22 /* N_SETT */); >> - dbxout_stab_value_label (XSTR (symbol, 0)); >> -#else >> - sorry ("global constructors not supported on this target"); >> -#endif >> -} > >> --- a/gcc/dbxout.h >> +++ /dev/null >> @@ -1,60 +0,0 @@ >> -/* dbxout.h - Various declarations for functions found in dbxout.cc > >> -extern void default_stabs_asm_out_destructor (rtx, int); >> -extern void default_stabs_asm_out_constructor (rtx, int); > > ... these two functions, 'default_stabs_asm_out_constructor', > 'default_stabs_asm_out_destructor' (specifically, now their 'sorry' > branches only) used to serve as default 'TARGET_ASM_CONSTRUCTOR', > 'TARGET_ASM_DESTRUCTOR' via... > >> --- a/gcc/target-def.h >> +++ b/gcc/target-def.h > > | #if !defined(TARGET_ASM_CONSTRUCTOR) && !defined(USE_COLLECT2) > | # ifdef CTORS_SECTION_ASM_OP > | # define TARGET_ASM_CONSTRUCTOR default_ctor_section_asm_out_constructor >> # else >> # ifdef TARGET_ASM_NAMED_SECTION >> # define TARGET_ASM_CONSTRUCTOR default_named_section_asm_out_constructor >> -# else >> -# define TARGET_ASM_CONSTRUCTOR default_stabs_asm_out_constructor >> # endif >> # endif >> #endif >> @@ -74,8 +72,6 @@ > | #if !defined(TARGET_ASM_DESTRUCTOR) && !defined(USE_COLLECT2) > | # ifdef DTORS_SECTION_ASM_OP > | # define TARGET_ASM_DESTRUCTOR default_dtor_section_asm_out_destructor >> # else >> # ifdef TARGET_ASM_NAMED_SECTION >> # define TARGET_ASM_DESTRUCTOR default_named_section_asm_out_destructor >> -# else >> -# define TARGET_ASM_DESTRUCTOR default_stabs_asm_out_destructor >> # endif >> # endif >> #endif > > ... this setup here (manually added some more context to the 'diff'). > > That is, if a back end was not 'USE_COLLECT2', nor manually defined > 'TARGET_ASM_CONSTRUCTOR', 'TARGET_ASM_DESTRUCTOR', or got pointed to the > respective 'default_[...]' functions due to 'CTORS_SECTION_ASM_OP', > 'DTORS_SECTION_ASM_OP', or 'TARGET_ASM_NAMED_SECTION', it got pointed to > 'default_stabs_asm_out_constructor', 'default_stabs_asm_out_destructor'. > These would emit 'sorry' for any global constructor/destructor they're > run into. > > This is now gone, and thus in such a back end configuration case > 'TARGET_ASM_CONSTRUCTOR', 'TARGET_ASM_DESTRUCTOR' don't get defined > anymore, and thus the subsequently following: > > #if !defined(TARGET_HAVE_CTORS_DTORS) > # if defined(TARGET_ASM_CONSTRUCTOR) && defined(TARGET_ASM_DESTRUCTOR) > # define TARGET_HAVE_CTORS_DTORS true > # endif > #endif > > ... doesn't define 'TARGET_HAVE_CTORS_DTORS' anymore, and thus per my > understanding, 'gcc/final.cc:rest_of_handle_final': > > if (DECL_STATIC_CONSTRUCTOR (current_function_decl) > && targetm.have_ctors_dtors) > targetm.asm_out.constructor (XEXP (DECL_RTL (current_function_decl), 0), > decl_init_priority_lookup > (current_function_decl)); > if (DECL_STATIC_DESTRUCTOR (current_function_decl) > && targetm.have_ctors_dtors) > targetm.asm_out.destructor (XEXP (DECL_RTL (current_function_decl), 0), > decl_fini_priority_lookup > (current_function_decl)); > > ... simply does nothing anymore for a 'DECL_STATIC_CONSTRUCTOR', > 'DECL_STATIC_DESTRUCTOR'. > > This, effectively, means that GCC/nvptx now suddenly appears to "support" > global constructors/destructors, which means that a ton of test cases now > erroneously PASS that previously used to FAIL: > > sorry, unimplemented: global constructors not supported on this target > > Of course, such support didn't magically happen due to > "STABS: remove -gstabs and -gxcoff functionality", so this is bad. And, > corresponding execution testing then regularly FAILs (due to the global > constructor/destructor functions never being invoked), for example: > > [-UNSUPPORTED:-]{+PASS:+} gcc.dg/initpri1.c {+(test for excess errors)+} > {+FAIL: gcc.dg/initpri1.c execution test+} > > [-UNSUPPORTED:-]{+PASS:+} g++.dg/special/conpr-1.C {+(test for excess errors)+} > {+FAIL: g++.dg/special/conpr-1.C execution test+} > > To restore the previous GCC/nvptx behavior, I seek permission to first > push the attached > "Restore default 'sorry' 'TARGET_ASM_CONSTRUCTOR', 'TARGET_ASM_DESTRUCTOR'". > For traceability, this simply restores the previous code, stripped down > to the bare minimum. > > As a next step, I'd then prepare a patch to re-work this thing, to make > it more obvious what's actually happening there. (Details to be done. > For example, do we want to continue to have 'sorry' functions called via > 'gcc/target-def.h', as done previously? Or, should that be an '#error' > case there, and the nvptx back end provides its own 'sorry' functions for > 'TARGET_ASM_CONSTRUCTOR', 'TARGET_ASM_DESTRUCTOR'? Or, emit 'sorry' in > 'gcc/final.cc:rest_of_handle_final'? Or, something else?) > > > In addition to 'gcc/config/nvptx/nvptx.cc' (as quoted above) three more > files did '#include "dbxout.h"', and thus potentially could use these > 'sorry' default functions. > > Per my testing of '--target=pdp11-aout' as well as > '--target=powerpc64-darwin', re-introducing '#include "dbxout.h"' is not > relevant for 'gcc/config/pdp11/pdp11.cc' as well as for > 'gcc/config/rs6000/rs6000-call.cc' and > 'gcc/config/rs6000/rs6000-logue.cc' (for 'TARGET_MACHO' only). > > Target pdp11-aout is not affected, because of 'gcc/config.gcc' setting: > > # On a.out targets, we need to use collect2. > case ${target} in > *-*-*aout*) > use_collect2=yes > ;; > esac > > Target powerpc64-darwin is not affected, because it defines > 'TARGET_ASM_NAMED_SECTION'. > > I'll thus remove those changes/TODO markers that are still present in the > attached > "Restore default 'sorry' 'TARGET_ASM_CONSTRUCTOR', 'TARGET_ASM_DESTRUCTOR'". > With that, OK to push? > > > 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 Hi Thomas. Thanks for the fix, really appreciated! Martin