| Message ID | 66f538cc.050a0220.1b4ceb.0a3f@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] libgcc, libstdc++: Make TU-local declarations in headers external linkage [PR115126] | expand |
On Thu, Sep 26, 2024 at 08:34:45PM +1000, Nathaniel Shead wrote: > --- a/libgcc/gthr-posix.h > +++ b/libgcc/gthr-posix.h > @@ -44,6 +44,21 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > # endif > #endif > > +#ifdef __has_attribute > +# if __has_attribute(__always_inline__) > +# define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) > +# endif > +#endif > +#ifndef __GTHREAD_ALWAYS_INLINE > +# define __GTHREAD_ALWAYS_INLINE > +#endif > + > +#ifdef __cplusplus > +# define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE > +#else > +# define __GTHREAD_INLINE static inline > +#endif Thanks. > @@ -182,22 +197,29 @@ __gthrw(pthread_setschedparam) > > #if defined(__FreeBSD__) || (defined(__sun) && defined(__svr4__)) > > -static volatile int __gthread_active = -1; > +#pragma GCC visibility push(hidden) > +__GTHREAD_INLINE volatile int * > +__gthread_active (void) > +{ > + static volatile int __gthread_active_var = -1; > + return &__gthread_active_var; > +} > +#pragma GCC visibility pop I think something like the above > -static void > +__GTHREAD_INLINE void > __gthread_trigger (void) > { > - __gthread_active = 1; > + *__gthread_active () = 1; > } > > -static inline int > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > static pthread_mutex_t __gthread_active_mutex = PTHREAD_MUTEX_INITIALIZER; > static pthread_once_t __gthread_active_once = PTHREAD_ONCE_INIT; is needed also around this one. > @@ -257,13 +279,15 @@ __gthrw2(__gthrw_(__pthread_key_create), > # define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel) > #endif > > -static inline int > +#pragma GCC visibility push(hidden) > +__GTHREAD_INLINE int > __gthread_active_p (void) > { > static void *const __gthread_active_ptr > = __extension__ (void *) >HR_ACTIVE_PROXY; > return __gthread_active_ptr != 0; > } > +#pragma GCC visibility pop And this one but you've added it to that one already. Otherwise LGTM for the libgcc side, will defer to Jon for libstdc++ side. Jakub
On Thu, 26 Sept 2024 at 13:07, Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Sep 26, 2024 at 08:34:45PM +1000, Nathaniel Shead wrote: > > --- a/libgcc/gthr-posix.h > > +++ b/libgcc/gthr-posix.h > > @@ -44,6 +44,21 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > > # endif > > #endif > > > > +#ifdef __has_attribute > > +# if __has_attribute(__always_inline__) > > +# define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) > > +# endif > > +#endif > > +#ifndef __GTHREAD_ALWAYS_INLINE > > +# define __GTHREAD_ALWAYS_INLINE > > +#endif > > + > > +#ifdef __cplusplus > > +# define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE > > +#else > > +# define __GTHREAD_INLINE static inline > > +#endif > > Thanks. > > > @@ -182,22 +197,29 @@ __gthrw(pthread_setschedparam) > > > > #if defined(__FreeBSD__) || (defined(__sun) && defined(__svr4__)) > > > > -static volatile int __gthread_active = -1; > > +#pragma GCC visibility push(hidden) > > +__GTHREAD_INLINE volatile int * > > +__gthread_active (void) > > +{ > > + static volatile int __gthread_active_var = -1; > > + return &__gthread_active_var; > > +} > > +#pragma GCC visibility pop > > I think something like the above > > > -static void > > +__GTHREAD_INLINE void > > __gthread_trigger (void) > > { > > - __gthread_active = 1; > > + *__gthread_active () = 1; > > } > > > > -static inline int > > +__GTHREAD_INLINE int > > __gthread_active_p (void) > > { > > static pthread_mutex_t __gthread_active_mutex = PTHREAD_MUTEX_INITIALIZER; > > static pthread_once_t __gthread_active_once = PTHREAD_ONCE_INIT; > > is needed also around this one. > > > @@ -257,13 +279,15 @@ __gthrw2(__gthrw_(__pthread_key_create), > > # define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel) > > #endif > > > > -static inline int > > +#pragma GCC visibility push(hidden) > > +__GTHREAD_INLINE int > > __gthread_active_p (void) > > { > > static void *const __gthread_active_ptr > > = __extension__ (void *) >HR_ACTIVE_PROXY; > > return __gthread_active_ptr != 0; > > } > > +#pragma GCC visibility pop > > And this one but you've added it to that one already. > > Otherwise LGTM for the libgcc side, will defer to Jon for libstdc++ side. The libstdc++ parts are OK too, thanks for working on this.
On 9/26/24 6:34 AM, Nathaniel Shead wrote: > On Thu, Sep 26, 2024 at 01:46:27PM +1000, Nathaniel Shead wrote: >> On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: >>> On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: >>>>>> And whether similarly we couldn't use >>>>>> __attribute__((__visibility__ ("hidden"))) on the static block scope >>>>>> vars for C++ (again, if compiler supports that), so that the changes >>>>>> don't affect ABI of C++ libraries. >>>>> >>>>> That sounds good too. >>>> >>>> Can you use visibility attributes on a local static? I get a warning >>>> that it's ignored. >>> >>> Indeed :( >>> >>> And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around >>> just the static block scope var definition does nothing. >>> If it is around the whole inline function though, then it seems to work. >>> Though, unsure if we want that around the whole header; wonder what it would >>> do with the weakrefs. >>> >>> Jakub >>> >> >> Thanks for the thoughts. WRT visibility, it looks like the main gthr.h >> surrounds the whole function in a >> >> #ifndef HIDE_EXPORTS >> #pragma GCC visibility push(default) >> #endif >> >> block, though I can't quite work out what the purpose of that is here >> (since everything is currently internal linkage to start with). >> >> But it sounds like doing something like >> >> #ifdef __has_attribute >> # if __has_attribute(__always_inline__) >> # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) >> # endif >> #endif >> #ifndef __GTHREAD_ALWAYS_INLINE >> # define __GTHREAD_ALWAYS_INLINE >> #endif >> >> #ifdef __cplusplus >> # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE >> #else >> # define __GTHREAD_INLINE static inline >> #endif >> >> and then marking maybe even just the new inline functions with >> visibility hidden should be OK? >> >> Nathaniel > > Here's a new patch that does this. Also since v1 it adds another two > internal linkage declarations I'd missed earlier from libstdc++, in > pstl; it turns out that <bits/stdc++.h> doesn't include <execution>. > > Bootstrapped and regtested on x86_64-pc-linux-gnu and > aarch64-unknown-linux-gnu, OK for trunk? > > -- >8 -- > > In C++20, modules streaming check for exposures of TU-local entities. > In general exposing internal linkage functions in a header is liable to > cause ODR violations in C++, and this is now detected in a module > context. > > This patch goes through and removes 'static' from many declarations > exposed through libstdc++ to prevent code like the following from > failing: > > export module M; > extern "C++" { > #include <bits/stdc++.h> > } > > Since gthreads is used from C as well, we need to choose whether to use > 'inline' or 'static inline' depending on whether we're compiling for C > or C++ (since the semantics of 'inline' are different between the > languages). Additionally we need to remove static global variables, so > we migrate these to function-local statics to avoid the ODR issues. Why function-local static rather than inline variable? > +++ b/libstdc++-v3/include/pstl/algorithm_impl.h > @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ > }); > } > > -constexpr auto __set_algo_cut_off = 1000; > +inline constexpr auto __set_algo_cut_off = 1000; > > +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h > @@ -22,7 +22,7 @@ namespace __unseq_backend > { > > // Expect vector width up to 64 (or 512 bit) > -const std::size_t __lane_size = 64; > +inline const std::size_t __lane_size = 64; These changes should not be necessary; the uses of these variables are not exposures under https://eel.is/c++draft/basic#link-14.4 Jason
On Fri, 27 Sept 2024 at 19:46, Jason Merrill <jason@redhat.com> wrote: > > On 9/26/24 6:34 AM, Nathaniel Shead wrote: > > On Thu, Sep 26, 2024 at 01:46:27PM +1000, Nathaniel Shead wrote: > >> On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: > >>> On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: > >>>>>> And whether similarly we couldn't use > >>>>>> __attribute__((__visibility__ ("hidden"))) on the static block scope > >>>>>> vars for C++ (again, if compiler supports that), so that the changes > >>>>>> don't affect ABI of C++ libraries. > >>>>> > >>>>> That sounds good too. > >>>> > >>>> Can you use visibility attributes on a local static? I get a warning > >>>> that it's ignored. > >>> > >>> Indeed :( > >>> > >>> And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around > >>> just the static block scope var definition does nothing. > >>> If it is around the whole inline function though, then it seems to work. > >>> Though, unsure if we want that around the whole header; wonder what it would > >>> do with the weakrefs. > >>> > >>> Jakub > >>> > >> > >> Thanks for the thoughts. WRT visibility, it looks like the main gthr.h > >> surrounds the whole function in a > >> > >> #ifndef HIDE_EXPORTS > >> #pragma GCC visibility push(default) > >> #endif > >> > >> block, though I can't quite work out what the purpose of that is here > >> (since everything is currently internal linkage to start with). > >> > >> But it sounds like doing something like > >> > >> #ifdef __has_attribute > >> # if __has_attribute(__always_inline__) > >> # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) > >> # endif > >> #endif > >> #ifndef __GTHREAD_ALWAYS_INLINE > >> # define __GTHREAD_ALWAYS_INLINE > >> #endif > >> > >> #ifdef __cplusplus > >> # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE > >> #else > >> # define __GTHREAD_INLINE static inline > >> #endif > >> > >> and then marking maybe even just the new inline functions with > >> visibility hidden should be OK? > >> > >> Nathaniel > > > > Here's a new patch that does this. Also since v1 it adds another two > > internal linkage declarations I'd missed earlier from libstdc++, in > > pstl; it turns out that <bits/stdc++.h> doesn't include <execution>. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu and > > aarch64-unknown-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > In C++20, modules streaming check for exposures of TU-local entities. > > In general exposing internal linkage functions in a header is liable to > > cause ODR violations in C++, and this is now detected in a module > > context. > > > > This patch goes through and removes 'static' from many declarations > > exposed through libstdc++ to prevent code like the following from > > failing: > > > > export module M; > > extern "C++" { > > #include <bits/stdc++.h> > > } > > > > Since gthreads is used from C as well, we need to choose whether to use > > 'inline' or 'static inline' depending on whether we're compiling for C > > or C++ (since the semantics of 'inline' are different between the > > languages). Additionally we need to remove static global variables, so > > we migrate these to function-local statics to avoid the ODR issues. > > Why function-local static rather than inline variable? We can make that conditional on __cplusplus but can we do that for C++98? With Clang too? > > > +++ b/libstdc++-v3/include/pstl/algorithm_impl.h > > @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ > > }); > > } > > > > -constexpr auto __set_algo_cut_off = 1000; > > +inline constexpr auto __set_algo_cut_off = 1000; > > > > +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h > > @@ -22,7 +22,7 @@ namespace __unseq_backend > > { > > > > // Expect vector width up to 64 (or 512 bit) > > -const std::size_t __lane_size = 64; > > +inline const std::size_t __lane_size = 64; > > These changes should not be necessary; the uses of these variables are > not exposures under https://eel.is/c++draft/basic#link-14.4 > > Jason >
On 9/27/24 3:38 PM, Jonathan Wakely wrote: > On Fri, 27 Sept 2024 at 19:46, Jason Merrill <jason@redhat.com> wrote: >> >> On 9/26/24 6:34 AM, Nathaniel Shead wrote: >>> On Thu, Sep 26, 2024 at 01:46:27PM +1000, Nathaniel Shead wrote: >>>> On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: >>>>> On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: >>>>>>>> And whether similarly we couldn't use >>>>>>>> __attribute__((__visibility__ ("hidden"))) on the static block scope >>>>>>>> vars for C++ (again, if compiler supports that), so that the changes >>>>>>>> don't affect ABI of C++ libraries. >>>>>>> >>>>>>> That sounds good too. >>>>>> >>>>>> Can you use visibility attributes on a local static? I get a warning >>>>>> that it's ignored. >>>>> >>>>> Indeed :( >>>>> >>>>> And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around >>>>> just the static block scope var definition does nothing. >>>>> If it is around the whole inline function though, then it seems to work. >>>>> Though, unsure if we want that around the whole header; wonder what it would >>>>> do with the weakrefs. >>>>> >>>>> Jakub >>>>> >>>> >>>> Thanks for the thoughts. WRT visibility, it looks like the main gthr.h >>>> surrounds the whole function in a >>>> >>>> #ifndef HIDE_EXPORTS >>>> #pragma GCC visibility push(default) >>>> #endif >>>> >>>> block, though I can't quite work out what the purpose of that is here >>>> (since everything is currently internal linkage to start with). >>>> >>>> But it sounds like doing something like >>>> >>>> #ifdef __has_attribute >>>> # if __has_attribute(__always_inline__) >>>> # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) >>>> # endif >>>> #endif >>>> #ifndef __GTHREAD_ALWAYS_INLINE >>>> # define __GTHREAD_ALWAYS_INLINE >>>> #endif >>>> >>>> #ifdef __cplusplus >>>> # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE >>>> #else >>>> # define __GTHREAD_INLINE static inline >>>> #endif >>>> >>>> and then marking maybe even just the new inline functions with >>>> visibility hidden should be OK? >>>> >>>> Nathaniel >>> >>> Here's a new patch that does this. Also since v1 it adds another two >>> internal linkage declarations I'd missed earlier from libstdc++, in >>> pstl; it turns out that <bits/stdc++.h> doesn't include <execution>. >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu and >>> aarch64-unknown-linux-gnu, OK for trunk? >>> >>> -- >8 -- >>> >>> In C++20, modules streaming check for exposures of TU-local entities. >>> In general exposing internal linkage functions in a header is liable to >>> cause ODR violations in C++, and this is now detected in a module >>> context. >>> >>> This patch goes through and removes 'static' from many declarations >>> exposed through libstdc++ to prevent code like the following from >>> failing: >>> >>> export module M; >>> extern "C++" { >>> #include <bits/stdc++.h> >>> } >>> >>> Since gthreads is used from C as well, we need to choose whether to use >>> 'inline' or 'static inline' depending on whether we're compiling for C >>> or C++ (since the semantics of 'inline' are different between the >>> languages). Additionally we need to remove static global variables, so >>> we migrate these to function-local statics to avoid the ODR issues. >> >> Why function-local static rather than inline variable? > > We can make that conditional on __cplusplus but can we do that for > C++98? With Clang too? Yes for both compilers, disabling -Wc++17-extensions. >>> +++ b/libstdc++-v3/include/pstl/algorithm_impl.h >>> @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ >>> }); >>> } >>> >>> -constexpr auto __set_algo_cut_off = 1000; >>> +inline constexpr auto __set_algo_cut_off = 1000; >>> >>> +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h >>> @@ -22,7 +22,7 @@ namespace __unseq_backend >>> { >>> >>> // Expect vector width up to 64 (or 512 bit) >>> -const std::size_t __lane_size = 64; >>> +inline const std::size_t __lane_size = 64; >> >> These changes should not be necessary; the uses of these variables are >> not exposures under https://eel.is/c++draft/basic#link-14.4 >> >> Jason >> >
On Fri, Sep 27, 2024 at 03:55:14PM -0400, Jason Merrill wrote: > On 9/27/24 3:38 PM, Jonathan Wakely wrote: > > On Fri, 27 Sept 2024 at 19:46, Jason Merrill <jason@redhat.com> wrote: > > > > > > On 9/26/24 6:34 AM, Nathaniel Shead wrote: > > > > On Thu, Sep 26, 2024 at 01:46:27PM +1000, Nathaniel Shead wrote: > > > > > On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: > > > > > > On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: > > > > > > > > > And whether similarly we couldn't use > > > > > > > > > __attribute__((__visibility__ ("hidden"))) on the static block scope > > > > > > > > > vars for C++ (again, if compiler supports that), so that the changes > > > > > > > > > don't affect ABI of C++ libraries. > > > > > > > > > > > > > > > > That sounds good too. > > > > > > > > > > > > > > Can you use visibility attributes on a local static? I get a warning > > > > > > > that it's ignored. > > > > > > > > > > > > Indeed :( > > > > > > > > > > > > And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around > > > > > > just the static block scope var definition does nothing. > > > > > > If it is around the whole inline function though, then it seems to work. > > > > > > Though, unsure if we want that around the whole header; wonder what it would > > > > > > do with the weakrefs. > > > > > > > > > > > > Jakub > > > > > > > > > > > > > > > > Thanks for the thoughts. WRT visibility, it looks like the main gthr.h > > > > > surrounds the whole function in a > > > > > > > > > > #ifndef HIDE_EXPORTS > > > > > #pragma GCC visibility push(default) > > > > > #endif > > > > > > > > > > block, though I can't quite work out what the purpose of that is here > > > > > (since everything is currently internal linkage to start with). > > > > > > > > > > But it sounds like doing something like > > > > > > > > > > #ifdef __has_attribute > > > > > # if __has_attribute(__always_inline__) > > > > > # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) > > > > > # endif > > > > > #endif > > > > > #ifndef __GTHREAD_ALWAYS_INLINE > > > > > # define __GTHREAD_ALWAYS_INLINE > > > > > #endif > > > > > > > > > > #ifdef __cplusplus > > > > > # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE > > > > > #else > > > > > # define __GTHREAD_INLINE static inline > > > > > #endif > > > > > > > > > > and then marking maybe even just the new inline functions with > > > > > visibility hidden should be OK? > > > > > > > > > > Nathaniel > > > > > > > > Here's a new patch that does this. Also since v1 it adds another two > > > > internal linkage declarations I'd missed earlier from libstdc++, in > > > > pstl; it turns out that <bits/stdc++.h> doesn't include <execution>. > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu and > > > > aarch64-unknown-linux-gnu, OK for trunk? > > > > > > > > -- >8 -- > > > > > > > > In C++20, modules streaming check for exposures of TU-local entities. > > > > In general exposing internal linkage functions in a header is liable to > > > > cause ODR violations in C++, and this is now detected in a module > > > > context. > > > > > > > > This patch goes through and removes 'static' from many declarations > > > > exposed through libstdc++ to prevent code like the following from > > > > failing: > > > > > > > > export module M; > > > > extern "C++" { > > > > #include <bits/stdc++.h> > > > > } > > > > > > > > Since gthreads is used from C as well, we need to choose whether to use > > > > 'inline' or 'static inline' depending on whether we're compiling for C > > > > or C++ (since the semantics of 'inline' are different between the > > > > languages). Additionally we need to remove static global variables, so > > > > we migrate these to function-local statics to avoid the ODR issues. > > > > > > Why function-local static rather than inline variable? > > > > We can make that conditional on __cplusplus but can we do that for > > C++98? With Clang too? > > Yes for both compilers, disabling -Wc++17-extensions. > Ah, I didn't realise that was possible. I've already merged the above patch but happy to test another one that changes this if that's preferred. > > > > +++ b/libstdc++-v3/include/pstl/algorithm_impl.h > > > > @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ > > > > }); > > > > } > > > > > > > > -constexpr auto __set_algo_cut_off = 1000; > > > > +inline constexpr auto __set_algo_cut_off = 1000; > > > > > > > > +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h > > > > @@ -22,7 +22,7 @@ namespace __unseq_backend > > > > { > > > > > > > > // Expect vector width up to 64 (or 512 bit) > > > > -const std::size_t __lane_size = 64; > > > > +inline const std::size_t __lane_size = 64; > > > > > > These changes should not be necessary; the uses of these variables are > > > not exposures under https://eel.is/c++draft/basic#link-14.4 > > > Right, forgot about that. Looks like I'll need to update my patch to support this then, perhaps by also eagerly folding constants in template declarations as is currently done for non-templates? > > > Jason > > > > > >
On 9/27/24 7:29 PM, Nathaniel Shead wrote: > On Fri, Sep 27, 2024 at 03:55:14PM -0400, Jason Merrill wrote: >> On 9/27/24 3:38 PM, Jonathan Wakely wrote: >>> On Fri, 27 Sept 2024 at 19:46, Jason Merrill <jason@redhat.com> wrote: >>>> >>>> On 9/26/24 6:34 AM, Nathaniel Shead wrote: >>>>> On Thu, Sep 26, 2024 at 01:46:27PM +1000, Nathaniel Shead wrote: >>>>>> On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: >>>>>>> On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: >>>>>>>>>> And whether similarly we couldn't use >>>>>>>>>> __attribute__((__visibility__ ("hidden"))) on the static block scope >>>>>>>>>> vars for C++ (again, if compiler supports that), so that the changes >>>>>>>>>> don't affect ABI of C++ libraries. >>>>>>>>> >>>>>>>>> That sounds good too. >>>>>>>> >>>>>>>> Can you use visibility attributes on a local static? I get a warning >>>>>>>> that it's ignored. >>>>>>> >>>>>>> Indeed :( >>>>>>> >>>>>>> And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around >>>>>>> just the static block scope var definition does nothing. >>>>>>> If it is around the whole inline function though, then it seems to work. >>>>>>> Though, unsure if we want that around the whole header; wonder what it would >>>>>>> do with the weakrefs. >>>>>>> >>>>>>> Jakub >>>>>>> >>>>>> >>>>>> Thanks for the thoughts. WRT visibility, it looks like the main gthr.h >>>>>> surrounds the whole function in a >>>>>> >>>>>> #ifndef HIDE_EXPORTS >>>>>> #pragma GCC visibility push(default) >>>>>> #endif >>>>>> >>>>>> block, though I can't quite work out what the purpose of that is here >>>>>> (since everything is currently internal linkage to start with). >>>>>> >>>>>> But it sounds like doing something like >>>>>> >>>>>> #ifdef __has_attribute >>>>>> # if __has_attribute(__always_inline__) >>>>>> # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) >>>>>> # endif >>>>>> #endif >>>>>> #ifndef __GTHREAD_ALWAYS_INLINE >>>>>> # define __GTHREAD_ALWAYS_INLINE >>>>>> #endif >>>>>> >>>>>> #ifdef __cplusplus >>>>>> # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE >>>>>> #else >>>>>> # define __GTHREAD_INLINE static inline >>>>>> #endif >>>>>> >>>>>> and then marking maybe even just the new inline functions with >>>>>> visibility hidden should be OK? >>>>>> >>>>>> Nathaniel >>>>> >>>>> Here's a new patch that does this. Also since v1 it adds another two >>>>> internal linkage declarations I'd missed earlier from libstdc++, in >>>>> pstl; it turns out that <bits/stdc++.h> doesn't include <execution>. >>>>> >>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu and >>>>> aarch64-unknown-linux-gnu, OK for trunk? >>>>> >>>>> -- >8 -- >>>>> >>>>> In C++20, modules streaming check for exposures of TU-local entities. >>>>> In general exposing internal linkage functions in a header is liable to >>>>> cause ODR violations in C++, and this is now detected in a module >>>>> context. >>>>> >>>>> This patch goes through and removes 'static' from many declarations >>>>> exposed through libstdc++ to prevent code like the following from >>>>> failing: >>>>> >>>>> export module M; >>>>> extern "C++" { >>>>> #include <bits/stdc++.h> >>>>> } >>>>> >>>>> Since gthreads is used from C as well, we need to choose whether to use >>>>> 'inline' or 'static inline' depending on whether we're compiling for C >>>>> or C++ (since the semantics of 'inline' are different between the >>>>> languages). Additionally we need to remove static global variables, so >>>>> we migrate these to function-local statics to avoid the ODR issues. >>>> >>>> Why function-local static rather than inline variable? >>> >>> We can make that conditional on __cplusplus but can we do that for >>> C++98? With Clang too? >> >> Yes for both compilers, disabling -Wc++17-extensions. > > Ah, I didn't realise that was possible. I've already merged the above > patch but happy to test another one that changes this if that's > preferred. Jonathan, do you have a preference? >>>>> +++ b/libstdc++-v3/include/pstl/algorithm_impl.h >>>>> @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ >>>>> }); >>>>> } >>>>> >>>>> -constexpr auto __set_algo_cut_off = 1000; >>>>> +inline constexpr auto __set_algo_cut_off = 1000; >>>>> >>>>> +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h >>>>> @@ -22,7 +22,7 @@ namespace __unseq_backend >>>>> { >>>>> >>>>> // Expect vector width up to 64 (or 512 bit) >>>>> -const std::size_t __lane_size = 64; >>>>> +inline const std::size_t __lane_size = 64; >>>> >>>> These changes should not be necessary; the uses of these variables are >>>> not exposures under https://eel.is/c++draft/basic#link-14.4 > > Right, forgot about that. Looks like I'll need to update my patch to > support this then, perhaps by also eagerly folding constants in template > declarations as is currently done for non-templates? mark_use handles folding constants from a containing function of a lambda; perhaps it would make sense to fold tu-locals there as well. Jason
On Sat, 28 Sept 2024 at 18:37, Jason Merrill <jason@redhat.com> wrote: > > On 9/27/24 7:29 PM, Nathaniel Shead wrote: > > On Fri, Sep 27, 2024 at 03:55:14PM -0400, Jason Merrill wrote: > >> On 9/27/24 3:38 PM, Jonathan Wakely wrote: > >>> On Fri, 27 Sept 2024 at 19:46, Jason Merrill <jason@redhat.com> wrote: > >>>> > >>>> On 9/26/24 6:34 AM, Nathaniel Shead wrote: > >>>>> On Thu, Sep 26, 2024 at 01:46:27PM +1000, Nathaniel Shead wrote: > >>>>>> On Wed, Sep 25, 2024 at 01:30:55PM +0200, Jakub Jelinek wrote: > >>>>>>> On Wed, Sep 25, 2024 at 12:18:07PM +0100, Jonathan Wakely wrote: > >>>>>>>>>> And whether similarly we couldn't use > >>>>>>>>>> __attribute__((__visibility__ ("hidden"))) on the static block scope > >>>>>>>>>> vars for C++ (again, if compiler supports that), so that the changes > >>>>>>>>>> don't affect ABI of C++ libraries. > >>>>>>>>> > >>>>>>>>> That sounds good too. > >>>>>>>> > >>>>>>>> Can you use visibility attributes on a local static? I get a warning > >>>>>>>> that it's ignored. > >>>>>>> > >>>>>>> Indeed :( > >>>>>>> > >>>>>>> And #pragma GCC visibility push(hidden)/#pragma GCC visibility pop around > >>>>>>> just the static block scope var definition does nothing. > >>>>>>> If it is around the whole inline function though, then it seems to work. > >>>>>>> Though, unsure if we want that around the whole header; wonder what it would > >>>>>>> do with the weakrefs. > >>>>>>> > >>>>>>> Jakub > >>>>>>> > >>>>>> > >>>>>> Thanks for the thoughts. WRT visibility, it looks like the main gthr.h > >>>>>> surrounds the whole function in a > >>>>>> > >>>>>> #ifndef HIDE_EXPORTS > >>>>>> #pragma GCC visibility push(default) > >>>>>> #endif > >>>>>> > >>>>>> block, though I can't quite work out what the purpose of that is here > >>>>>> (since everything is currently internal linkage to start with). > >>>>>> > >>>>>> But it sounds like doing something like > >>>>>> > >>>>>> #ifdef __has_attribute > >>>>>> # if __has_attribute(__always_inline__) > >>>>>> # define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) > >>>>>> # endif > >>>>>> #endif > >>>>>> #ifndef __GTHREAD_ALWAYS_INLINE > >>>>>> # define __GTHREAD_ALWAYS_INLINE > >>>>>> #endif > >>>>>> > >>>>>> #ifdef __cplusplus > >>>>>> # define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE > >>>>>> #else > >>>>>> # define __GTHREAD_INLINE static inline > >>>>>> #endif > >>>>>> > >>>>>> and then marking maybe even just the new inline functions with > >>>>>> visibility hidden should be OK? > >>>>>> > >>>>>> Nathaniel > >>>>> > >>>>> Here's a new patch that does this. Also since v1 it adds another two > >>>>> internal linkage declarations I'd missed earlier from libstdc++, in > >>>>> pstl; it turns out that <bits/stdc++.h> doesn't include <execution>. > >>>>> > >>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu and > >>>>> aarch64-unknown-linux-gnu, OK for trunk? > >>>>> > >>>>> -- >8 -- > >>>>> > >>>>> In C++20, modules streaming check for exposures of TU-local entities. > >>>>> In general exposing internal linkage functions in a header is liable to > >>>>> cause ODR violations in C++, and this is now detected in a module > >>>>> context. > >>>>> > >>>>> This patch goes through and removes 'static' from many declarations > >>>>> exposed through libstdc++ to prevent code like the following from > >>>>> failing: > >>>>> > >>>>> export module M; > >>>>> extern "C++" { > >>>>> #include <bits/stdc++.h> > >>>>> } > >>>>> > >>>>> Since gthreads is used from C as well, we need to choose whether to use > >>>>> 'inline' or 'static inline' depending on whether we're compiling for C > >>>>> or C++ (since the semantics of 'inline' are different between the > >>>>> languages). Additionally we need to remove static global variables, so > >>>>> we migrate these to function-local statics to avoid the ODR issues. > >>>> > >>>> Why function-local static rather than inline variable? > >>> > >>> We can make that conditional on __cplusplus but can we do that for > >>> C++98? With Clang too? > >> > >> Yes for both compilers, disabling -Wc++17-extensions. > > > > Ah, I didn't realise that was possible. I've already merged the above > > patch but happy to test another one that changes this if that's > > preferred. > > Jonathan, do you have a preference? Let's use an inline variable. A function-local static requires __cxa_guard_acquire, which (for some targets, including the ones affected by this change) uses __gthread_active_p which will recursively re-enter the variable's initialization. So something like: #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wc++17-extensions" inline volatile int __gthread_active = -1; #pragma GCC diagnostic pop This code misuses volatile (obligatory https://isvolatileusefulwiththreads.in/c++/ link) where it should use atomics to load and store that shared variable. But that can be fixed later. > > >>>>> +++ b/libstdc++-v3/include/pstl/algorithm_impl.h > >>>>> @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ > >>>>> }); > >>>>> } > >>>>> > >>>>> -constexpr auto __set_algo_cut_off = 1000; > >>>>> +inline constexpr auto __set_algo_cut_off = 1000; > >>>>> > >>>>> +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h > >>>>> @@ -22,7 +22,7 @@ namespace __unseq_backend > >>>>> { > >>>>> > >>>>> // Expect vector width up to 64 (or 512 bit) > >>>>> -const std::size_t __lane_size = 64; > >>>>> +inline const std::size_t __lane_size = 64; > >>>> > >>>> These changes should not be necessary; the uses of these variables are > >>>> not exposures under https://eel.is/c++draft/basic#link-14.4 > > > > Right, forgot about that. Looks like I'll need to update my patch to > > support this then, perhaps by also eagerly folding constants in template > > declarations as is currently done for non-templates? > > mark_use handles folding constants from a containing function of a > lambda; perhaps it would make sense to fold tu-locals there as well. > > Jason >
On Tue, Oct 01, 2024 at 11:10:03AM +0100, Jonathan Wakely wrote: > Let's use an inline variable. A function-local static requires > __cxa_guard_acquire, which (for some targets, including the ones > affected by this change) uses __gthread_active_p which will > recursively re-enter the variable's initialization. > > So something like: > > #pragma GCC diagnostic push > #pragma GCC diagnostic ignored "-Wc++17-extensions" > inline volatile int __gthread_active = -1; > #pragma GCC diagnostic pop Note, only for #ifdef __cplusplus, for C there is no such thing as inline variables and in that case it should use static volatile int __ghtread_active = -1; instead. Jakub
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h index 82e8f9ffcf6..e30786a1802 100644 --- a/libgcc/gthr-posix.h +++ b/libgcc/gthr-posix.h @@ -44,6 +44,21 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # endif #endif +#ifdef __has_attribute +# if __has_attribute(__always_inline__) +# define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) +# endif +#endif +#ifndef __GTHREAD_ALWAYS_INLINE +# define __GTHREAD_ALWAYS_INLINE +#endif + +#ifdef __cplusplus +# define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE +#else +# define __GTHREAD_INLINE static inline +#endif + typedef pthread_t __gthread_t; typedef pthread_key_t __gthread_key_t; typedef pthread_once_t __gthread_once_t; @@ -182,22 +197,29 @@ __gthrw(pthread_setschedparam) #if defined(__FreeBSD__) || (defined(__sun) && defined(__svr4__)) -static volatile int __gthread_active = -1; +#pragma GCC visibility push(hidden) +__GTHREAD_INLINE volatile int * +__gthread_active (void) +{ + static volatile int __gthread_active_var = -1; + return &__gthread_active_var; +} +#pragma GCC visibility pop -static void +__GTHREAD_INLINE void __gthread_trigger (void) { - __gthread_active = 1; + *__gthread_active () = 1; } -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { static pthread_mutex_t __gthread_active_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_once_t __gthread_active_once = PTHREAD_ONCE_INIT; /* Avoid reading __gthread_active twice on the main code path. */ - int __gthread_active_latest_value = __gthread_active; + int __gthread_active_latest_value = *__gthread_active (); /* This test is not protected to avoid taking a lock on the main code path so every update of __gthread_active in a threaded program must @@ -214,10 +236,10 @@ __gthread_active_p (void) } /* Make sure we'll never enter this block again. */ - if (__gthread_active < 0) - __gthread_active = 0; + if (*__gthread_active () < 0) + *__gthread_active () = 0; - __gthread_active_latest_value = __gthread_active; + __gthread_active_latest_value = *__gthread_active (); } return __gthread_active_latest_value != 0; @@ -257,13 +279,15 @@ __gthrw2(__gthrw_(__pthread_key_create), # define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel) #endif -static inline int +#pragma GCC visibility push(hidden) +__GTHREAD_INLINE int __gthread_active_p (void) { static void *const __gthread_active_ptr = __extension__ (void *) >HR_ACTIVE_PROXY; return __gthread_active_ptr != 0; } +#pragma GCC visibility pop #endif /* FreeBSD or Solaris */ @@ -288,20 +312,27 @@ __gthread_active_p (void) #if defined(__hppa__) && defined(__hpux__) -static volatile int __gthread_active = -1; +#pragma GCC visibility push(hidden) +__GTHREAD_INLINE volatile int * +__gthread_active (void) +{ + static volatile int __gthread_active_var = -1; + return &__gthread_active_var; +} +#pragma GCC visibility pop -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { /* Avoid reading __gthread_active twice on the main code path. */ - int __gthread_active_latest_value = __gthread_active; + int __gthread_active_latest_value = *__gthread_active (); size_t __s; if (__builtin_expect (__gthread_active_latest_value < 0, 0)) { pthread_default_stacksize_np (0, &__s); - __gthread_active = __s ? 1 : 0; - __gthread_active_latest_value = __gthread_active; + *__gthread_active () = __s ? 1 : 0; + __gthread_active_latest_value = *__gthread_active (); } return __gthread_active_latest_value != 0; @@ -309,7 +340,7 @@ __gthread_active_p (void) #else /* not hppa-hpux */ -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { return 1; @@ -669,44 +700,44 @@ __gthread_objc_condition_signal (objc_condition_t condition) #else /* _LIBOBJC */ -static inline int +__GTHREAD_INLINE int __gthread_create (__gthread_t *__threadid, void *(*__func) (void*), void *__args) { return __gthrw_(pthread_create) (__threadid, NULL, __func, __args); } -static inline int +__GTHREAD_INLINE int __gthread_join (__gthread_t __threadid, void **__value_ptr) { return __gthrw_(pthread_join) (__threadid, __value_ptr); } -static inline int +__GTHREAD_INLINE int __gthread_detach (__gthread_t __threadid) { return __gthrw_(pthread_detach) (__threadid); } -static inline int +__GTHREAD_INLINE int __gthread_equal (__gthread_t __t1, __gthread_t __t2) { return __gthrw_(pthread_equal) (__t1, __t2); } -static inline __gthread_t +__GTHREAD_INLINE __gthread_t __gthread_self (void) { return __gthrw_(pthread_self) (); } -static inline int +__GTHREAD_INLINE int __gthread_yield (void) { return __gthrw_(sched_yield) (); } -static inline int +__GTHREAD_INLINE int __gthread_once (__gthread_once_t *__once, void (*__func) (void)) { if (__gthread_active_p ()) @@ -715,38 +746,38 @@ __gthread_once (__gthread_once_t *__once, void (*__func) (void)) return -1; } -static inline int +__GTHREAD_INLINE int __gthread_key_create (__gthread_key_t *__key, void (*__dtor) (void *)) { return __gthrw_(pthread_key_create) (__key, __dtor); } -static inline int +__GTHREAD_INLINE int __gthread_key_delete (__gthread_key_t __key) { return __gthrw_(pthread_key_delete) (__key); } -static inline void * +__GTHREAD_INLINE void * __gthread_getspecific (__gthread_key_t __key) { return __gthrw_(pthread_getspecific) (__key); } -static inline int +__GTHREAD_INLINE int __gthread_setspecific (__gthread_key_t __key, const void *__ptr) { return __gthrw_(pthread_setspecific) (__key, __ptr); } -static inline void +__GTHREAD_INLINE void __gthread_mutex_init_function (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) __gthrw_(pthread_mutex_init) (__mutex, NULL); } -static inline int +__GTHREAD_INLINE int __gthread_mutex_destroy (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -755,7 +786,7 @@ __gthread_mutex_destroy (__gthread_mutex_t *__mutex) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_lock (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -764,7 +795,7 @@ __gthread_mutex_lock (__gthread_mutex_t *__mutex) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_trylock (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -774,7 +805,7 @@ __gthread_mutex_trylock (__gthread_mutex_t *__mutex) } #if _GTHREAD_USE_MUTEX_TIMEDLOCK -static inline int +__GTHREAD_INLINE int __gthread_mutex_timedlock (__gthread_mutex_t *__mutex, const __gthread_time_t *__abs_timeout) { @@ -785,7 +816,7 @@ __gthread_mutex_timedlock (__gthread_mutex_t *__mutex, } #endif -static inline int +__GTHREAD_INLINE int __gthread_mutex_unlock (__gthread_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -796,7 +827,7 @@ __gthread_mutex_unlock (__gthread_mutex_t *__mutex) #if !defined( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) \ || defined(_GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC) -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) { if (__gthread_active_p ()) @@ -818,20 +849,20 @@ __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) } #endif -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_lock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_lock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_trylock (__mutex); } #if _GTHREAD_USE_MUTEX_TIMEDLOCK -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex, const __gthread_time_t *__abs_timeout) { @@ -839,20 +870,20 @@ __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex, } #endif -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_unlock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_destroy (__mutex); } #ifdef _GTHREAD_USE_COND_INIT_FUNC -static inline void +__GTHREAD_INLINE void __gthread_cond_init_function (__gthread_cond_t *__cond) { if (__gthread_active_p ()) @@ -860,46 +891,46 @@ __gthread_cond_init_function (__gthread_cond_t *__cond) } #endif -static inline int +__GTHREAD_INLINE int __gthread_cond_broadcast (__gthread_cond_t *__cond) { return __gthrw_(pthread_cond_broadcast) (__cond); } -static inline int +__GTHREAD_INLINE int __gthread_cond_signal (__gthread_cond_t *__cond) { return __gthrw_(pthread_cond_signal) (__cond); } -static inline int +__GTHREAD_INLINE int __gthread_cond_wait (__gthread_cond_t *__cond, __gthread_mutex_t *__mutex) { return __gthrw_(pthread_cond_wait) (__cond, __mutex); } -static inline int +__GTHREAD_INLINE int __gthread_cond_timedwait (__gthread_cond_t *__cond, __gthread_mutex_t *__mutex, const __gthread_time_t *__abs_timeout) { return __gthrw_(pthread_cond_timedwait) (__cond, __mutex, __abs_timeout); } -static inline int +__GTHREAD_INLINE int __gthread_cond_wait_recursive (__gthread_cond_t *__cond, __gthread_recursive_mutex_t *__mutex) { return __gthread_cond_wait (__cond, __mutex); } -static inline int +__GTHREAD_INLINE int __gthread_cond_destroy (__gthread_cond_t* __cond) { return __gthrw_(pthread_cond_destroy) (__cond); } #ifndef __cplusplus -static inline int +__GTHREAD_INLINE int __gthread_rwlock_rdlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -908,7 +939,7 @@ __gthread_rwlock_rdlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_tryrdlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -917,7 +948,7 @@ __gthread_rwlock_tryrdlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_wrlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -926,7 +957,7 @@ __gthread_rwlock_wrlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_trywrlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -935,7 +966,7 @@ __gthread_rwlock_trywrlock (__gthread_rwlock_t *__rwlock) return 0; } -static inline int +__GTHREAD_INLINE int __gthread_rwlock_unlock (__gthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -947,4 +978,7 @@ __gthread_rwlock_unlock (__gthread_rwlock_t *__rwlock) #endif /* _LIBOBJC */ +#undef __GTHREAD_INLINE +#undef __GTHREAD_ALWAYS_INLINE + #endif /* ! GCC_GTHR_POSIX_H */ diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h index 8ee6b170840..2a799ad3d3a 100644 --- a/libgcc/gthr-single.h +++ b/libgcc/gthr-single.h @@ -38,6 +38,21 @@ typedef int __gthread_recursive_mutex_t; #define __GTHREAD_MUTEX_INIT_FUNCTION(mx) do {} while (0) #define __GTHREAD_RECURSIVE_MUTEX_INIT 0 +#ifdef __has_attribute +# if __has_attribute(__always_inline__) +# define __GTHREAD_ALWAYS_INLINE __attribute__((__always_inline__)) +# endif +#endif +#ifndef __GTHREAD_ALWAYS_INLINE +# define __GTHREAD_ALWAYS_INLINE +#endif + +#ifdef __cplusplus +# define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE +#else +# define __GTHREAD_INLINE static inline +#endif + #define UNUSED __attribute__((__unused__)) #ifdef _LIBOBJC @@ -207,85 +222,85 @@ __gthread_objc_condition_signal (objc_condition_t condition UNUSED) #else /* _LIBOBJC */ -static inline int +__GTHREAD_INLINE int __gthread_active_p (void) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_once (__gthread_once_t *__once UNUSED, void (*__func) (void) UNUSED) { return 0; } -static inline int UNUSED +__GTHREAD_INLINE int UNUSED __gthread_key_create (__gthread_key_t *__key UNUSED, void (*__func) (void *) UNUSED) { return 0; } -static int UNUSED +__GTHREAD_INLINE int UNUSED __gthread_key_delete (__gthread_key_t __key UNUSED) { return 0; } -static inline void * +__GTHREAD_INLINE void * __gthread_getspecific (__gthread_key_t __key UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_setspecific (__gthread_key_t __key UNUSED, const void *__v UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_destroy (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_lock (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_trylock (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_mutex_unlock (__gthread_mutex_t *__mutex UNUSED) { return 0; } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_lock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_lock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_trylock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_trylock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_unlock (__mutex); } -static inline int +__GTHREAD_INLINE int __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) { return __gthread_mutex_destroy (__mutex); @@ -294,5 +309,7 @@ __gthread_recursive_mutex_destroy (__gthread_recursive_mutex_t *__mutex) #endif /* _LIBOBJC */ #undef UNUSED +#undef __GTHREAD_INLINE +#undef __GTHREAD_ALWAYS_INLINE #endif /* ! GCC_GTHR_SINGLE_H */ diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index 13273afa9d2..9b1c2b4eb3d 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -1160,9 +1160,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cpp_variable_templates template<typename _Tp> - static constexpr bool __is_shared_ptr = false; + constexpr bool __is_shared_ptr = false; template<typename _Tp> - static constexpr bool __is_shared_ptr<shared_ptr<_Tp>> = true; + constexpr bool __is_shared_ptr<shared_ptr<_Tp>> = true; #endif /// @} relates shared_ptr diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h index edcff78bff9..182173aa857 100644 --- a/libstdc++-v3/include/bits/unique_ptr.h +++ b/libstdc++-v3/include/bits/unique_ptr.h @@ -1157,9 +1157,9 @@ namespace __detail #if __cpp_variable_templates template<typename _Tp> - static constexpr bool __is_unique_ptr = false; + constexpr bool __is_unique_ptr = false; template<typename _Tp, typename _Del> - static constexpr bool __is_unique_ptr<unique_ptr<_Tp, _Del>> = true; + constexpr bool __is_unique_ptr<unique_ptr<_Tp, _Del>> = true; #endif /// @} group pointer_abstractions diff --git a/libstdc++-v3/include/pstl/algorithm_impl.h b/libstdc++-v3/include/pstl/algorithm_impl.h index aec8c7484e7..1403b02280f 100644 --- a/libstdc++-v3/include/pstl/algorithm_impl.h +++ b/libstdc++-v3/include/pstl/algorithm_impl.h @@ -2890,7 +2890,7 @@ __pattern_includes(__parallel_tag<_IsVector> __tag, _ExecutionPolicy&& __exec, _ }); } -constexpr auto __set_algo_cut_off = 1000; +inline constexpr auto __set_algo_cut_off = 1000; template <class _IsVector, class _ExecutionPolicy, class _ForwardIterator1, class _ForwardIterator2, class _OutputIterator, class _Compare, class _SizeFunction, class _SetOP> diff --git a/libstdc++-v3/include/pstl/unseq_backend_simd.h b/libstdc++-v3/include/pstl/unseq_backend_simd.h index f3c38fbbbc2..58a2c0c0adb 100644 --- a/libstdc++-v3/include/pstl/unseq_backend_simd.h +++ b/libstdc++-v3/include/pstl/unseq_backend_simd.h @@ -22,7 +22,7 @@ namespace __unseq_backend { // Expect vector width up to 64 (or 512 bit) -const std::size_t __lane_size = 64; +inline const std::size_t __lane_size = 64; template <class _Iterator, class _DifferenceType, class _Function> _Iterator diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future index b1e6a5354d8..3c2baaa1f6e 100644 --- a/libstdc++-v3/include/std/future +++ b/libstdc++-v3/include/std/future @@ -1528,7 +1528,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Signature, typename _Fn, typename _Alloc = std::allocator<int>> - static shared_ptr<__future_base::_Task_state_base<_Signature>> + shared_ptr<__future_base::_Task_state_base<_Signature>> __create_task_state(_Fn&& __fn, const _Alloc& __a = _Alloc()) { typedef typename decay<_Fn>::type _Fn2; diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex index f1bb5d7fe92..9bf98c0b040 100644 --- a/libstdc++-v3/include/std/shared_mutex +++ b/libstdc++-v3/include/std/shared_mutex @@ -74,7 +74,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #ifdef __gthrw #define _GLIBCXX_GTHRW(name) \ __gthrw(pthread_ ## name); \ - static inline int \ + inline int \ __glibcxx_ ## name (pthread_rwlock_t *__rwlock) \ { \ if (__gthread_active_p ()) \ @@ -90,7 +90,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION # ifndef PTHREAD_RWLOCK_INITIALIZER _GLIBCXX_GTHRW(rwlock_destroy) __gthrw(pthread_rwlock_init); - static inline int + inline int __glibcxx_rwlock_init (pthread_rwlock_t *__rwlock) { if (__gthread_active_p ()) @@ -101,7 +101,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION # endif # if _GTHREAD_USE_MUTEX_TIMEDLOCK __gthrw(pthread_rwlock_timedrdlock); - static inline int + inline int __glibcxx_rwlock_timedrdlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { @@ -111,7 +111,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return 0; } __gthrw(pthread_rwlock_timedwrlock); - static inline int + inline int __glibcxx_rwlock_timedwrlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { @@ -122,33 +122,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } # endif #else - static inline int + inline int __glibcxx_rwlock_rdlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_rdlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_tryrdlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_tryrdlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_wrlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_wrlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_trywrlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_trywrlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_unlock (pthread_rwlock_t *__rwlock) { return pthread_rwlock_unlock (__rwlock); } - static inline int + inline int __glibcxx_rwlock_destroy(pthread_rwlock_t *__rwlock) { return pthread_rwlock_destroy (__rwlock); } - static inline int + inline int __glibcxx_rwlock_init(pthread_rwlock_t *__rwlock) { return pthread_rwlock_init (__rwlock, NULL); } # if _GTHREAD_USE_MUTEX_TIMEDLOCK - static inline int + inline int __glibcxx_rwlock_timedrdlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { return pthread_rwlock_timedrdlock (__rwlock, __ts); } - static inline int + inline int __glibcxx_rwlock_timedwrlock (pthread_rwlock_t *__rwlock, const timespec *__ts) { return pthread_rwlock_timedwrlock (__rwlock, __ts); }