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 C42863858282 for ; Fri, 4 Nov 2022 16:31:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C42863858282 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=1667579481; 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=zvHwtCIaQ7aGQiVTDTQjAbm6O/PHBLbADN93YO3+Yws=; b=aPmkxwzvEqK2Popkux+CDq0kbm9pYtZp0l56VY+oIIin9rUd8m0/xML3FkT6TNAF2Uik/+ oiAZj/GziGjsWXi7ukmH7FR8sXD4Q58xoCeIYYQoboobfsYWMcFmY2uv30A3vJMwXFy5iX 3CF7nFy2Sm5xz2CZNqrLGlXGhefblbQ= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-553-azB-gT6HNC2-rQbKNxJxcA-1; Fri, 04 Nov 2022 12:31:15 -0400 X-MC-Unique: azB-gT6HNC2-rQbKNxJxcA-1 Received: by mail-qt1-f199.google.com with SMTP id cm12-20020a05622a250c00b003a521f66e8eso4075993qtb.17 for ; Fri, 04 Nov 2022 09:31:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zvHwtCIaQ7aGQiVTDTQjAbm6O/PHBLbADN93YO3+Yws=; b=2ECdecMnjNb+ZwOJIbHaXus97J61u/fmAC1YIIYWAxAcKQR5BuJmnY7HhzNujnEKL5 Buybart+0khDX6XQ0d37sF8fYayMueladFmtgFqcGmRWJcgYPRmnE4uwZM4lgBc5R0ya rR4cZxrLcTsLzs1BkviQHi645QukEYk7tn4nsLzghqpbTlfaRZv+2UEJhTi0UEHqbY5i 652mCX5gG+rrS2S4Zy017e8K3H67hr6uI2vpUHGL2/t/UrnuvJZQjn57qqsuBwrEtig8 bveQjmEzJpmstyVCJuaZaMAmCgDUJbZo8UUuM8dNWAwl1w6LHtpjiuM6bSaM9JR8Clg+ pGog== X-Gm-Message-State: ACrzQf0mmrKubTtonuzRwtZRrVu/zofm7//adAJ7PrhdgqAoHnFEGV6Q w90Ra560aPgRBfhRFuwxTNzefb4PgTXNDCKoqGdb/KVs+8LSbaw9mKFFRsQh6gxGPMe6sf51/zL Rgvgvhk4HeKSvdCo= X-Received: by 2002:a05:622a:252:b0:3a5:73a:1aa3 with SMTP id c18-20020a05622a025200b003a5073a1aa3mr30299748qtx.482.1667579473901; Fri, 04 Nov 2022 09:31:13 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7DoPjrNS7UkP6tyhNRGJ0rMj9Q835VdYqGh6PT8FIDttxJSGw5RsBz1H23cQOB023hv3uZ/g== X-Received: by 2002:a05:622a:252:b0:3a5:73a:1aa3 with SMTP id c18-20020a05622a025200b003a5073a1aa3mr30299718qtx.482.1667579473539; Fri, 04 Nov 2022 09:31:13 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id u23-20020ac87517000000b0039a372fbaa5sm2647817qtq.69.2022.11.04.09.31.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Nov 2022 09:31:12 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Fri, 4 Nov 2022 12:31:12 -0400 (EDT) To: Jonathan Wakely cc: Patrick Palka , gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org, jason@redhat.com Subject: Re: [PATCH 2/2] libstdc++: Move stream initialization into compiled library [PR44952] In-Reply-To: Message-ID: References: <20221104150525.2968778-1-ppalka@redhat.com> <20221104150525.2968778-2-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-14.5 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=unavailable 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, 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: -- >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