On Tue, Oct 01, 2024 at 12:36:21PM +0200, Jakub Jelinek wrote:
> 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
>
So something like this maybe; bootstrapped and regtested on
x86_64-pc-linux-gnu and aarch64-unknown-linux-gnu, OK for trunk?
Or maybe it would be clearer to do the #ifdef __cplusplus here locally
rather than introducing a new __GTHREAD_VAR macro?
-- >8 --
Subject: [PATCH] libgcc: Use inline variable instead of function-local static
To support C++ ODR rules but remain compatible with C, I had initially
changed these variables to be function-local statics rather than global
internal-linkage decls.
However, both GCC and Clang support using namespace-scope 'inline'
variables even in pre-C++17 (with a warning), so let's simplify and do
that instead. For C we instead revert back to using an internal-linkage
variable.
PR c++/115126
libgcc/ChangeLog:
* gthr-posix.h (__GTHREAD_VAR): New macro.
(__gthread_active): Be a possibly-inline variable instead of a
function. Ignore any resultant diagnostics.
(__gthread_trigger): __gthread_active is now a variable.
(__gthread_active_p): Likewise.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
libgcc/gthr-posix.h | 39 +++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
index 478bcf4cee6..b24992a539f 100644
--- a/libgcc/gthr-posix.h
+++ b/libgcc/gthr-posix.h
@@ -55,8 +55,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If
not, see
#ifdef __cplusplus
# define __GTHREAD_INLINE inline __GTHREAD_ALWAYS_INLINE
+# define __GTHREAD_VAR inline
#else
# define __GTHREAD_INLINE static inline
+# define __GTHREAD_VAR static
#endif
typedef pthread_t __gthread_t;
@@ -198,18 +200,16 @@ __gthrw(pthread_setschedparam)
#if defined(__FreeBSD__) || (defined(__sun) && defined(__svr4__))
#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 diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions"
+__GTHREAD_VAR volatile int * __gthread_active = -1;
+#pragma GCC diagnostic pop
#pragma GCC visibility pop
__GTHREAD_INLINE void
__gthread_trigger (void)
{
- *__gthread_active () = 1;
+ *__gthread_active = 1;
}
#pragma GCC visibility push(hidden)
@@ -220,7 +220,7 @@ __gthread_active_p (void)
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
@@ -237,10 +237,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;
@@ -315,26 +315,24 @@ __gthread_active_p (void)
#if defined(__hppa__) && defined(__hpux__)
#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 diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions"
+__GTHREAD_VAR volatile int * __gthread_active = -1;
+#pragma GCC diagnostic pop
#pragma GCC visibility pop
__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;
@@ -980,6 +978,7 @@ __gthread_rwlock_unlock (__gthread_rwlock_t *__rwlock)
#endif /* _LIBOBJC */
+#undef __GTHREAD_VAR
#undef __GTHREAD_INLINE
#undef __GTHREAD_ALWAYS_INLINE
--
2.46.0
