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
