From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4206A384B112 for ; Tue, 23 Apr 2024 16:05:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4206A384B112 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4206A384B112 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713888314; cv=none; b=lcBvyqWde8J2xqE+UQt7kIVY/S7zfcWUKljxt7fmpbnpY82QavIVaejpqYHrO+pwD3+ZSdRURZ4scsEwJ6Tt4Pzv/UFv7PGGS3NR8tjJc6ikvolkymXSO0WB3SNHqqoQReFOzLKatmD4yo8Ot2C/SWzkeOrv0m4QyQ1ngHLGPRo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713888314; c=relaxed/simple; bh=KIyPUYnuvFTGzVAkw4CVW9a8ehBnzbQBFE1Ywf63LuY=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=Vp3P16EWtHN+vwciHx9UPLS2uPHk+RTdtWOhb/5wmxKEU2+dqq/Qaw5LBX9Ix9/Zn7R780UtlrOwNvAygBIHKfAzAUApXtWUnjqpTffOlSzewinlTp1hrqIgr2fCq/doC7gHiZ0pBpGLV/tEbp41ekZWoANa8UIomjcQXTvev4g= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713888301; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=63ofzw99jl246YoHU4I4IfIQPWV3Xm+HUA0Bm+vf0nc=; b=ZyMo28UQVHmYtS5Op9WbMUqjDjnbf+X9e+XvlM3MOAfvcvdZLKUCn5gCs9Ixk1nNuT5XTX PDrPM3SNg3bh4lPuuU0vA8qSdLKoZHwTY+JmHAFE890PPxL3MQqS92fzE3q/5RhCfS9oAe pp430FzVKPB4V5szFfgquBpOqW7P6M8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-394-4-ckBdkKNzCOb6mBsDdbWA-1; Tue, 23 Apr 2024 12:05:00 -0400 X-MC-Unique: 4-ckBdkKNzCOb6mBsDdbWA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 07CA2811007; Tue, 23 Apr 2024 16:05:00 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.45.224.5]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AD50D40EE0E; Tue, 23 Apr 2024 16:04:59 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 43NG4wVD932927 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 23 Apr 2024 18:04:58 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 43NG4v2A932926; Tue, 23 Apr 2024 18:04:57 +0200 Date: Tue, 23 Apr 2024 18:04:57 +0200 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Jan Hubicka , Richard Biener , Patrick Palka , Jonathan Wakely Subject: Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208] Message-ID: Reply-To: Jakub Jelinek References: <5fe9db40-4bff-4be4-a434-6509133f26e3@redhat.com> MIME-Version: 1.0 In-Reply-To: <5fe9db40-4bff-4be4-a434-6509133f26e3@redhat.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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 Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote: > > > The following testcase regressed with Marek's r14-5979 change, > > > when pr113208_0.C is compiled where the ctor is marked constexpr, > > > we no longer perform this optimization, where > > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the > > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and > > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it, > > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in > > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same > > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in > > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group. > > This seems like an ABI bug that could use a non-LTO testcase. Well, except for the issues it causes to LTO I think it is compatible, worst case we get the body of the ctor duplicated in the executable and the linker picks some of the weak symbols as the symbol definitions. Anyway, I've added a non-LTO testcase for that in the patch below. > Hmm, cloning the bodies and then discarding them later seems like more extra > work than creating the cgraph nodes. So, I've tried to handle that in tentative_decl_linkage, like that function already handles functions declared inline except for implicit template instantiations. If we expect that import_export_decl will do comdat_linkage for the ctor later on do it right away. That fixes the testcases too, but seems to regress +FAIL: libstdc++-abi/abi_check on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from libstdc++.so.6: _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev Will need to study why that happened, it might be that it was ok because I think the filesystem stuff is unlike the rest compiled with no exported templates, but would need at least some hacks in libstdc++ to preserve previously exported symbols. Still, feels like a risky change this late if it wouldn't break ABI of other libraries. 2024-04-23 Jakub Jelinek PR lto/113208 * decl2.cc (tentative_decl_linkage): Use comdat_linkage also for implicit instantiations of maybe in charge ctors/dtors if -fimplicit-templates or -fimplicit-inline-templates and -fweak and target supports aliases. * g++.dg/abi/comdat2.C: New test. * g++.dg/lto/pr113208_0.C: New test. * g++.dg/lto/pr113208_1.C: New file. * g++.dg/lto/pr113208.h: New file. --- gcc/cp/decl2.cc.jj 2024-04-22 15:16:55.328548807 +0200 +++ gcc/cp/decl2.cc 2024-04-23 09:52:18.993250442 +0200 @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl) to mark the functions at this point. */ if (DECL_DECLARED_INLINE_P (decl) && (!DECL_IMPLICIT_INSTANTIATION (decl) - || DECL_DEFAULTED_FN (decl))) + || DECL_DEFAULTED_FN (decl) + /* For implicit instantiations of cdtors, + if import_export_decl would use comdat linkage, + make sure to use it right away, so that maybe_clone_body + can use aliases. See PR113208. */ + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl) + && (flag_implicit_templates + || flag_implicit_inline_templates) + && flag_weak + && TARGET_SUPPORTS_ALIASES))) { /* This function must have external linkage, as otherwise DECL_INTERFACE_KNOWN would have been --- gcc/testsuite/g++.dg/abi/comdat2.C.jj 2024-04-23 10:04:28.485964610 +0200 +++ gcc/testsuite/g++.dg/abi/comdat2.C 2024-04-23 10:05:24.757171194 +0200 @@ -0,0 +1,26 @@ +// PR lto/113208 +// { dg-do compile { target { c++11 && { *-*-*gnu* } } } } +// { dg-additional-options "-O2 -fkeep-inline-functions" } +// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } } +// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } } +// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } } + +template +struct A { + int foo () const; + A (int, int); +}; +template +struct B : A { + constexpr B (const B &x) : A (1, x.foo ()) {} + B () : A (1, 2) {} +}; +struct C; +struct D : B {}; +void bar (D); + +void +baz (D x) +{ + bar (x); +} --- gcc/testsuite/g++.dg/lto/pr113208.h.jj 2024-04-23 09:44:07.523179110 +0200 +++ gcc/testsuite/g++.dg/lto/pr113208.h 2024-04-23 09:44:07.523179110 +0200 @@ -0,0 +1,10 @@ +template struct _Vector_base { + int g() const; + _Vector_base(int, int); +}; +template +struct vector : _Vector_base<_Tp> { + CONSTEXPR vector(const vector &__x) + : _Vector_base<_Tp>(1, __x.g()) {} + vector() : _Vector_base<_Tp>(1, 2) {} +}; --- gcc/testsuite/g++.dg/lto/pr113208_1.C.jj 2024-04-23 09:44:07.523179110 +0200 +++ gcc/testsuite/g++.dg/lto/pr113208_1.C 2024-04-23 09:44:07.523179110 +0200 @@ -0,0 +1,6 @@ +#define CONSTEXPR +#include "pr113208.h" + +struct QualityValue; +vector values1; +vector values{values1}; --- gcc/testsuite/g++.dg/lto/pr113208_0.C.jj 2024-04-23 09:44:07.523179110 +0200 +++ gcc/testsuite/g++.dg/lto/pr113208_0.C 2024-04-23 09:44:07.523179110 +0200 @@ -0,0 +1,13 @@ +// { dg-lto-do link } +// { dg-lto-options { {-O1 -std=c++20 -flto}} } +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" } +// { dg-require-linker-plugin "" } + +#define CONSTEXPR constexpr +#include "pr113208.h" + +struct QualityValue; +struct k : vector {}; + +void m(k); +void n(k i) { m(i); } Jakub