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.129.124]) by sourceware.org (Postfix) with ESMTPS id 75BC53858414 for ; Fri, 4 Nov 2022 16:42:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 75BC53858414 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667580162; h=from:from: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=0kBg1yrjix3eLpR48TdmqphzmCBW1NY6RSy3T3Fz1Dg=; b=RXhPBg2IUfjIRLIY/YiRyHqFc7pQn1tvVDdbyis+57TxUFSzm8ylfN/xIV+mLbV/H1YkgM xn2eDDShlGcMaqegcP65fX9rkO5KFUHl7P6llJUhdslk0IH2zCO310S/GHzQgkIb8f1EY7 8n9QFJq2ZKjbZDvnK79qZGlM/dz6XpU= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-547-5UnmyRsQMieTTx3SQ2cduw-1; Fri, 04 Nov 2022 12:42:41 -0400 X-MC-Unique: 5UnmyRsQMieTTx3SQ2cduw-1 Received: by mail-ed1-f70.google.com with SMTP id z9-20020a05640235c900b0046358415c4fso3945828edc.9 for ; Fri, 04 Nov 2022 09:42:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=0kBg1yrjix3eLpR48TdmqphzmCBW1NY6RSy3T3Fz1Dg=; b=bAemu0Pl3da5+yosBOR3YGVW4g+FpmRhXknX7IPJdIvrxUy3VgwKXPQuCqtr0N1t0W YnSDiEwmqT8tQBn9o5OPGHibybeuyE56RYx+4xyOsEpdtTKKurUF1c1GmhiWCOibYRo/ BEmKzl7LD48BndlObbugKcSXWRGRdG56J6IKdIE6kefda1Xvy3ceIeo8F7qWRtqtr29A vI4T/Mqy/itZQanJi9dXQ21C8wCgUXJSRLu+bKJXZQ5hTqN1UgUcF0D0UU9cZzE/5utM BEi8aJCKxadiZRpQxzigw6PTbn/QLErrpoihcmBiueZ8gjLstAoHwj4S2jREqg/HuY/3 6wSg== X-Gm-Message-State: ACrzQf1NI8+G2xOO3OfsPzpbK/8pukIY/4QvB9tFcchmTq95Xhwqg8Z0 knUXfOFj7rAb71vVWNr4WwWUCvMocITVWQIRkIVnSzf04vcHUjvj0qRBEjT5mm8yjeLDNMzCNUj QFb9U9o/MJSP1q4obdpUS083QDgbyrTQ= X-Received: by 2002:a17:906:9bcd:b0:7ae:2679:c47 with SMTP id de13-20020a1709069bcd00b007ae26790c47mr7000002ejc.353.1667580159511; Fri, 04 Nov 2022 09:42:39 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7h2Mo7a/i/aCXwUYvChnOyKRJUwFgTNFko81dI9xX+jiM6zRbkdHnTWGcXI+SZoqXSlUvLocSqmZ6y14kqRU8= X-Received: by 2002:a17:906:9bcd:b0:7ae:2679:c47 with SMTP id de13-20020a1709069bcd00b007ae26790c47mr6999971ejc.353.1667580159099; Fri, 04 Nov 2022 09:42:39 -0700 (PDT) MIME-Version: 1.0 References: <20221104150525.2968778-1-ppalka@redhat.com> <20221104150525.2968778-2-ppalka@redhat.com> In-Reply-To: From: Jonathan Wakely Date: Fri, 4 Nov 2022 16:42:28 +0000 Message-ID: Subject: Re: [PATCH 2/2] libstdc++: Move stream initialization into compiled library [PR44952] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org, jason@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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 Fri, 4 Nov 2022 at 16:31, Patrick Palka wrote: > > On Fri, 4 Nov 2022, Jonathan Wakely wrote: > > > On Fri, 4 Nov 2022 at 15:08, Patrick Palka via Libstdc++ > > wrote: > > > > > > This patch moves the global object for constructing the standard streams > > > out from and into the compiled library on targets that support > > > the init_priority attribute. This means that no longer > > > introduces a separate global constructor in each TU that includes it. > > > > > > We can do this only if the init_priority attribute is supported because > > > we need to force that the stream initialization runs first before any > > > user-defined global initializer in programs that that use a static > > > libstdc++.a. > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look right? > > > Unfortunately I don't have access to a system that truly doesn't support > > > init priorities, so I instead tested that situation by artificially > > > disabling the init_priority attribute on x86_64. > > > > > > PR libstdc++/44952 > > > PR libstdc++/98108 > > > > > > libstdc++-v3/ChangeLog: > > > > > > * include/bits/c++config (_GLIBCXX_HAS_ATTRIBUTE): Define. > > > (_GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY): Define. > > > * include/std/iostream (__ioinit): Define only if init_priority > > > attribute isn't usable. > > > * src/c++98/ios_init.cc (__ioinit): Define here instead if > > > the init_priority is usable. > > > * src/c++98/ios_base_init.h: New file. > > > --- > > > libstdc++-v3/include/bits/c++config | 12 ++++++++++++ > > > libstdc++-v3/include/std/iostream | 4 ++++ > > > libstdc++-v3/src/c++98/ios_base_init.h | 9 +++++++++ > > > libstdc++-v3/src/c++98/ios_init.cc | 4 ++++ > > > 4 files changed, 29 insertions(+) > > > create mode 100644 libstdc++-v3/src/c++98/ios_base_init.h > > > > > > diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config > > > index 50406066afe..f93076191d9 100644 > > > --- a/libstdc++-v3/include/bits/c++config > > > +++ b/libstdc++-v3/include/bits/c++config > > > @@ -837,6 +837,18 @@ namespace __gnu_cxx > > > > > > #undef _GLIBCXX_HAS_BUILTIN > > > > > > +#ifdef __has_attribute > > > > Do we need this? > > I think all the compilers we support implemented this long ago (clang > > had it before gcc, and Intel has it for gcc compat, and any others had > > better have it by now too). > > > > So we can just use #if __has_attribute directly, instead of defining > > these extra macros. > > Sounds good, I wasn't sure if we can assume support for __has_attribute > or not. > > > > > > +# define _GLIBCXX_HAS_ATTRIBUTE(B) __has_attribute(B) > > > +#else > > > +# define _GLIBCXX_HAS_ATTRIBUTE(B) 0 > > > +#endif > > > + > > > +#if _GLIBCXX_HAS_ATTRIBUTE(init_priority) > > > +# define _GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY 1 > > > +#endif > > > + > > > +#undef _GLIBCXX_HAS_ATTRIBUTE > > > + > > > // Mark code that should be ignored by the compiler, but seen by Doxygen. > > > #define _GLIBCXX_DOXYGEN_ONLY(X) > > > > > > diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream > > > index 70318a45891..5eaa9755d9a 100644 > > > --- a/libstdc++-v3/include/std/iostream > > > +++ b/libstdc++-v3/include/std/iostream > > > @@ -73,7 +73,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > ///@} > > > > > > // For construction of filebuffers for cout, cin, cerr, clog et. al. > > > + // When the init_priority attribute is usable, we do this initialization > > > + // in the compiled library instead (see src/c++98/ios_init.cc). > > > +#if !_GLIBCXX_HAVE_ATTRIBUTE_INIT_PRIORITY > > > static ios_base::Init __ioinit; > > > +#endif > > > > > > _GLIBCXX_END_NAMESPACE_VERSION > > > } // namespace > > > diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h > > > new file mode 100644 > > > index 00000000000..f3087d1da3c > > > --- /dev/null > > > +++ b/libstdc++-v3/src/c++98/ios_base_init.h > > > @@ -0,0 +1,9 @@ > > > +// This is only in a header so we can use the system_header pragma, > > > +// to suppress the warning caused by using a reserved init_priority. > > > +#pragma GCC system_header > > > > Ugh, that's annoying. > > Yeah :( we employ the same workaround in > src/c++17/{memory_resource.cc,default_resource.h} too. > > Here's v2 which gets rid of the new macros, adds more comments and other > minor improvements. Only smoke tested so far, full bootstrap and > regtesting in progress: OK for trunk assuming testing passes. Thanks for working on it. > > -- >8 -- > > Subject: [PATCH 2/2] libstdc++: Move stream initialization into compiled > library [PR44952] > > This patch moves the global object for constructing the standard streams > out from and into the compiled library on targets that support > init priorities. This means that no longer introduces a > separate global constructor in each TU that includes it. > > We can do this only if the init_priority attribute is supported because > we need a way to force the stream initialization to run first before any > user-defined global initializer, particularly for programs that use a > static libstdc++.a. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look right? > Unfortunately I don't have access to a system that truly doesn't support > init priorities, so I instead tested that situation by artificially > disabling the init_priority attribute on x86_64. > > PR libstdc++/44952 > PR libstdc++/98108 > > libstdc++-v3/ChangeLog: > > * include/std/iostream (__ioinit): No longer define here if > init_priority attribute is usable. > * src/c++98/ios_init.cc (__ioinit): Define here instead if > the init_priority is usable. > * src/c++98/ios_base_init.h: New file. > --- > libstdc++-v3/include/std/iostream | 4 ++++ > libstdc++-v3/src/c++98/ios_base_init.h | 12 ++++++++++++ > libstdc++-v3/src/c++98/ios_init.cc | 2 ++ > 3 files changed, 18 insertions(+) > create mode 100644 libstdc++-v3/src/c++98/ios_base_init.h > > diff --git a/libstdc++-v3/include/std/iostream b/libstdc++-v3/include/std/iostream > index 70318a45891..ff78e1cfb87 100644 > --- a/libstdc++-v3/include/std/iostream > +++ b/libstdc++-v3/include/std/iostream > @@ -73,7 +73,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > ///@} > > // For construction of filebuffers for cout, cin, cerr, clog et. al. > + // When the init_priority attribute is usable, we do this initialization > + // in the compiled library instead (src/c++98/ios_init.cc). > +#if !__has_attribute(__init_priority__) > static ios_base::Init __ioinit; > +#endif > > _GLIBCXX_END_NAMESPACE_VERSION > } // namespace > diff --git a/libstdc++-v3/src/c++98/ios_base_init.h b/libstdc++-v3/src/c++98/ios_base_init.h > new file mode 100644 > index 00000000000..fe45e0e98a9 > --- /dev/null > +++ b/libstdc++-v3/src/c++98/ios_base_init.h > @@ -0,0 +1,12 @@ > +// This is only in a header so we can use the system_header pragma, > +// to suppress the warning caused by using a reserved init_priority. > +#pragma GCC system_header > + > +// If the target supports init priorities, set up a static object in the > +// compiled library to perform the initialization once and > +// sufficiently early (so that the initialization runs first before any > +// other global constructor when staticaly linking with libstdc++.a), > +// instead of having to do so in each TU that includes . > +#if __has_attribute(init_priority) > +static ios_base::Init __ioinit __attribute__((init_priority(90))); > +#endif > diff --git a/libstdc++-v3/src/c++98/ios_init.cc b/libstdc++-v3/src/c++98/ios_init.cc > index 1b5132f1c2d..4016fcab785 100644 > --- a/libstdc++-v3/src/c++98/ios_init.cc > +++ b/libstdc++-v3/src/c++98/ios_init.cc > @@ -75,6 +75,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > extern wostream wclog; > #endif > > +#include "ios_base_init.h" > + > ios_base::Init::Init() > { > if (__gnu_cxx::__exchange_and_add_dispatch(&_S_refcount, 1) == 0) > -- > 2.38.1.385.g3b08839926 >