* [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V
@ 2022-04-07 18:46 Palmer Dabbelt
2022-04-14 15:08 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2022-04-07 18:46 UTC (permalink / raw)
To: gcc-patches
The RISC-V port requires libatomic to be linked in order to resolve
various atomic functions, which results in builds that have
"--with-libstdcxx-lock-policy=auto" defaulting to mutex-based locks.
Changing this to direct atomics breaks the ABI, this forces the auto
detection mutex-based atomics on RISC-V in order to avoid a silent ABI
break for users.
See Bug 84568 for more discussion. In the long run there may be a way
to get the higher-performance atomics without an ABI flag day, but
that's going to be a much more complicated operation. We don't even
have support for the inline atomics yet, but given that some folks have
been discussing hacks to make these libatomic routines appear implicitly
it seems prudent to just turn off the automatic detection for RISC-V.
libstdc++-v3/ChangeLog
* acinclude.md (GLIBCXX_ENABLE_LOCK_POLICY): Force auto to mutex
for RISC-V.
---
I haven't even built this one, as I'm sure there's a better way to do it
then sticking some more C code in there.
---
libstdc++-v3/acinclude.m4 | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index f53461c85a5..945c0c66f8d 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3612,6 +3612,9 @@ AC_DEFUN([GLIBCXX_ENABLE_LOCK_POLICY], [
dnl Why don't we check 8-byte CAS for sparc64, where _Atomic_word is long?!
dnl New targets should only check for CAS for the _Atomic_word type.
AC_TRY_COMPILE([
+ #if defined __riscv
+ # error "Defaulting to mutex-based locks for ABI compatibility"
+ #endif
#if ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
# error "No 2-byte compare-and-swap"
#elif ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V
2022-04-07 18:46 [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V Palmer Dabbelt
@ 2022-04-14 15:08 ` Jonathan Wakely
2022-04-14 15:18 ` Palmer Dabbelt
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2022-04-14 15:08 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: gcc-patches, libstdc++
On 07/04/22 11:46 -0700, Palmer Dabbelt wrote:
>The RISC-V port requires libatomic to be linked in order to resolve
>various atomic functions, which results in builds that have
>"--with-libstdcxx-lock-policy=auto" defaulting to mutex-based locks.
>Changing this to direct atomics breaks the ABI, this forces the auto
>detection mutex-based atomics on RISC-V in order to avoid a silent ABI
>break for users.
>
>See Bug 84568 for more discussion. In the long run there may be a way
>to get the higher-performance atomics without an ABI flag day, but
>that's going to be a much more complicated operation. We don't even
>have support for the inline atomics yet, but given that some folks have
>been discussing hacks to make these libatomic routines appear implicitly
>it seems prudent to just turn off the automatic detection for RISC-V.
>
>libstdc++-v3/ChangeLog
>
> * acinclude.md (GLIBCXX_ENABLE_LOCK_POLICY): Force auto to mutex
> for RISC-V.
As documented at https://gcc.gnu.org/lists.html all patches for
libstdc++ need to go to the libstdc++ list as well as gcc-patches
(otherwise I won't see them).
We'd usually do something like:
case "${host}" in
*-*-riscv) libstdcxx_atomic_lock_policy=mutex ;;
*-*-*) AC_TRY_COMPILE([ ... ],,[],[])
esac
but this way is simpler. If we add more customization for other
targets we can reconsider using the 'case "${host}"' form.
So this is OK for trunk, modulo regenerating libstdc++-v3/configure
with this change. Let me know if you want me to do that regen for you
(or commit the whole thing for you).
>---
>
>I haven't even built this one, as I'm sure there's a better way to do it
>then sticking some more C code in there.
>---
> libstdc++-v3/acinclude.m4 | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
>index f53461c85a5..945c0c66f8d 100644
>--- a/libstdc++-v3/acinclude.m4
>+++ b/libstdc++-v3/acinclude.m4
>@@ -3612,6 +3612,9 @@ AC_DEFUN([GLIBCXX_ENABLE_LOCK_POLICY], [
> dnl Why don't we check 8-byte CAS for sparc64, where _Atomic_word is long?!
> dnl New targets should only check for CAS for the _Atomic_word type.
> AC_TRY_COMPILE([
>+ #if defined __riscv
>+ # error "Defaulting to mutex-based locks for ABI compatibility"
>+ #endif
> #if ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
> # error "No 2-byte compare-and-swap"
> #elif ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V
2022-04-14 15:08 ` Jonathan Wakely
@ 2022-04-14 15:18 ` Palmer Dabbelt
2022-04-14 15:22 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2022-04-14 15:18 UTC (permalink / raw)
To: jwakely; +Cc: gcc-patches, libstdc++
On Thu, 14 Apr 2022 08:08:17 PDT (-0700), jwakely@redhat.com wrote:
> On 07/04/22 11:46 -0700, Palmer Dabbelt wrote:
>>The RISC-V port requires libatomic to be linked in order to resolve
>>various atomic functions, which results in builds that have
>>"--with-libstdcxx-lock-policy=auto" defaulting to mutex-based locks.
>>Changing this to direct atomics breaks the ABI, this forces the auto
>>detection mutex-based atomics on RISC-V in order to avoid a silent ABI
>>break for users.
>>
>>See Bug 84568 for more discussion. In the long run there may be a way
>>to get the higher-performance atomics without an ABI flag day, but
>>that's going to be a much more complicated operation. We don't even
>>have support for the inline atomics yet, but given that some folks have
>>been discussing hacks to make these libatomic routines appear implicitly
>>it seems prudent to just turn off the automatic detection for RISC-V.
>>
>>libstdc++-v3/ChangeLog
>>
>> * acinclude.md (GLIBCXX_ENABLE_LOCK_POLICY): Force auto to mutex
>> for RISC-V.
>
> As documented at https://gcc.gnu.org/lists.html all patches for
> libstdc++ need to go to the libstdc++ list as well as gcc-patches
> (otherwise I won't see them).
Thanks, I'll try to remember to look next time.
> We'd usually do something like:
>
> case "${host}" in
> *-*-riscv) libstdcxx_atomic_lock_policy=mutex ;;
> *-*-*) AC_TRY_COMPILE([ ... ],,[],[])
> esac
>
> but this way is simpler. If we add more customization for other
> targets we can reconsider using the 'case "${host}"' form.
Ya, that's kind of where I came to as well -- the proper autoconf flavor
would scale way better, but hopefully nobody else makes this mistake and
thus we don't need to worry about that.
I'm fine with either way (though I think we'd need a "riscv*" there, to
match riscv32 and riscv64?), so if you want to swap it over (or have me
re-spin this) it's no big deal on my end -- also fine, as per below,
with you just committing this ;)
> So this is OK for trunk, modulo regenerating libstdc++-v3/configure
> with this change. Let me know if you want me to do that regen for you
> (or commit the whole thing for you).
That'd be great, thanks! It usually takes me a while to get all the
autotools versions lined up (we just got new machines at the office),
that way I won't have to do so.
>
>
>>---
>>
>>I haven't even built this one, as I'm sure there's a better way to do it
>>then sticking some more C code in there.
>>---
>> libstdc++-v3/acinclude.m4 | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>>diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
>>index f53461c85a5..945c0c66f8d 100644
>>--- a/libstdc++-v3/acinclude.m4
>>+++ b/libstdc++-v3/acinclude.m4
>>@@ -3612,6 +3612,9 @@ AC_DEFUN([GLIBCXX_ENABLE_LOCK_POLICY], [
>> dnl Why don't we check 8-byte CAS for sparc64, where _Atomic_word is long?!
>> dnl New targets should only check for CAS for the _Atomic_word type.
>> AC_TRY_COMPILE([
>>+ #if defined __riscv
>>+ # error "Defaulting to mutex-based locks for ABI compatibility"
>>+ #endif
>> #if ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
>> # error "No 2-byte compare-and-swap"
>> #elif ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V
2022-04-14 15:18 ` Palmer Dabbelt
@ 2022-04-14 15:22 ` Jonathan Wakely
2022-04-14 15:24 ` Palmer Dabbelt
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2022-04-14 15:22 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: gcc Patches, libstdc++
On Thu, 14 Apr 2022 at 16:18, Palmer Dabbelt wrote:
>
> On Thu, 14 Apr 2022 08:08:17 PDT (-0700), jwakely@redhat.com wrote:
> > On 07/04/22 11:46 -0700, Palmer Dabbelt wrote:
> >>The RISC-V port requires libatomic to be linked in order to resolve
> >>various atomic functions, which results in builds that have
> >>"--with-libstdcxx-lock-policy=auto" defaulting to mutex-based locks.
> >>Changing this to direct atomics breaks the ABI, this forces the auto
> >>detection mutex-based atomics on RISC-V in order to avoid a silent ABI
> >>break for users.
> >>
> >>See Bug 84568 for more discussion. In the long run there may be a way
> >>to get the higher-performance atomics without an ABI flag day, but
> >>that's going to be a much more complicated operation. We don't even
> >>have support for the inline atomics yet, but given that some folks have
> >>been discussing hacks to make these libatomic routines appear implicitly
> >>it seems prudent to just turn off the automatic detection for RISC-V.
> >>
> >>libstdc++-v3/ChangeLog
> >>
> >> * acinclude.md (GLIBCXX_ENABLE_LOCK_POLICY): Force auto to mutex
> >> for RISC-V.
> >
> > As documented at https://gcc.gnu.org/lists.html all patches for
> > libstdc++ need to go to the libstdc++ list as well as gcc-patches
> > (otherwise I won't see them).
>
> Thanks, I'll try to remember to look next time.
>
> > We'd usually do something like:
> >
> > case "${host}" in
> > *-*-riscv) libstdcxx_atomic_lock_policy=mutex ;;
> > *-*-*) AC_TRY_COMPILE([ ... ],,[],[])
> > esac
> >
> > but this way is simpler. If we add more customization for other
> > targets we can reconsider using the 'case "${host}"' form.
>
> Ya, that's kind of where I came to as well -- the proper autoconf flavor
> would scale way better, but hopefully nobody else makes this mistake and
> thus we don't need to worry about that.
<nod>
> I'm fine with either way (though I think we'd need a "riscv*" there, to
> match riscv32 and riscv64?), so if you want to swap it over (or have me
> re-spin this) it's no big deal on my end -- also fine, as per below,
> with you just committing this ;)
Yeah, I figured *-*-riscv probably wasn't right, so that's another
reason to prefer your approach.
>
> > So this is OK for trunk, modulo regenerating libstdc++-v3/configure
> > with this change. Let me know if you want me to do that regen for you
> > (or commit the whole thing for you).
>
> That'd be great, thanks! It usually takes me a while to get all the
> autotools versions lined up (we just got new machines at the office),
> that way I won't have to do so.
No problem, I can regen+push for you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V
2022-04-14 15:22 ` Jonathan Wakely
@ 2022-04-14 15:24 ` Palmer Dabbelt
2022-04-14 16:39 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2022-04-14 15:24 UTC (permalink / raw)
To: jwakely; +Cc: gcc-patches, libstdc++
On Thu, 14 Apr 2022 08:22:05 PDT (-0700), jwakely@redhat.com wrote:
> On Thu, 14 Apr 2022 at 16:18, Palmer Dabbelt wrote:
>>
>> On Thu, 14 Apr 2022 08:08:17 PDT (-0700), jwakely@redhat.com wrote:
>> > On 07/04/22 11:46 -0700, Palmer Dabbelt wrote:
>> >>The RISC-V port requires libatomic to be linked in order to resolve
>> >>various atomic functions, which results in builds that have
>> >>"--with-libstdcxx-lock-policy=auto" defaulting to mutex-based locks.
>> >>Changing this to direct atomics breaks the ABI, this forces the auto
>> >>detection mutex-based atomics on RISC-V in order to avoid a silent ABI
>> >>break for users.
>> >>
>> >>See Bug 84568 for more discussion. In the long run there may be a way
>> >>to get the higher-performance atomics without an ABI flag day, but
>> >>that's going to be a much more complicated operation. We don't even
>> >>have support for the inline atomics yet, but given that some folks have
>> >>been discussing hacks to make these libatomic routines appear implicitly
>> >>it seems prudent to just turn off the automatic detection for RISC-V.
>> >>
>> >>libstdc++-v3/ChangeLog
>> >>
>> >> * acinclude.md (GLIBCXX_ENABLE_LOCK_POLICY): Force auto to mutex
>> >> for RISC-V.
>> >
>> > As documented at https://gcc.gnu.org/lists.html all patches for
>> > libstdc++ need to go to the libstdc++ list as well as gcc-patches
>> > (otherwise I won't see them).
>>
>> Thanks, I'll try to remember to look next time.
>>
>> > We'd usually do something like:
>> >
>> > case "${host}" in
>> > *-*-riscv) libstdcxx_atomic_lock_policy=mutex ;;
>> > *-*-*) AC_TRY_COMPILE([ ... ],,[],[])
>> > esac
>> >
>> > but this way is simpler. If we add more customization for other
>> > targets we can reconsider using the 'case "${host}"' form.
>>
>> Ya, that's kind of where I came to as well -- the proper autoconf flavor
>> would scale way better, but hopefully nobody else makes this mistake and
>> thus we don't need to worry about that.
>
> <nod>
>
>> I'm fine with either way (though I think we'd need a "riscv*" there, to
>> match riscv32 and riscv64?), so if you want to swap it over (or have me
>> re-spin this) it's no big deal on my end -- also fine, as per below,
>> with you just committing this ;)
>
> Yeah, I figured *-*-riscv probably wasn't right, so that's another
> reason to prefer your approach.
>
>
>>
>> > So this is OK for trunk, modulo regenerating libstdc++-v3/configure
>> > with this change. Let me know if you want me to do that regen for you
>> > (or commit the whole thing for you).
>>
>> That'd be great, thanks! It usually takes me a while to get all the
>> autotools versions lined up (we just got new machines at the office),
>> that way I won't have to do so.
>
> No problem, I can regen+push for you.
Great, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V
2022-04-14 15:24 ` Palmer Dabbelt
@ 2022-04-14 16:39 ` Jonathan Wakely
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2022-04-14 16:39 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: gcc Patches, libstdc++
On Thu, 14 Apr 2022 at 16:24, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Thu, 14 Apr 2022 08:22:05 PDT (-0700), jwakely@redhat.com wrote:
> > On Thu, 14 Apr 2022 at 16:18, Palmer Dabbelt wrote:
> >>
> >> On Thu, 14 Apr 2022 08:08:17 PDT (-0700), jwakely@redhat.com wrote:
> >> > On 07/04/22 11:46 -0700, Palmer Dabbelt wrote:
> >> >>The RISC-V port requires libatomic to be linked in order to resolve
> >> >>various atomic functions, which results in builds that have
> >> >>"--with-libstdcxx-lock-policy=auto" defaulting to mutex-based locks.
> >> >>Changing this to direct atomics breaks the ABI, this forces the auto
> >> >>detection mutex-based atomics on RISC-V in order to avoid a silent ABI
> >> >>break for users.
> >> >>
> >> >>See Bug 84568 for more discussion. In the long run there may be a way
> >> >>to get the higher-performance atomics without an ABI flag day, but
> >> >>that's going to be a much more complicated operation. We don't even
> >> >>have support for the inline atomics yet, but given that some folks have
> >> >>been discussing hacks to make these libatomic routines appear implicitly
> >> >>it seems prudent to just turn off the automatic detection for RISC-V.
> >> >>
> >> >>libstdc++-v3/ChangeLog
> >> >>
> >> >> * acinclude.md (GLIBCXX_ENABLE_LOCK_POLICY): Force auto to mutex
> >> >> for RISC-V.
> >> >
> >> > As documented at https://gcc.gnu.org/lists.html all patches for
> >> > libstdc++ need to go to the libstdc++ list as well as gcc-patches
> >> > (otherwise I won't see them).
> >>
> >> Thanks, I'll try to remember to look next time.
> >>
> >> > We'd usually do something like:
> >> >
> >> > case "${host}" in
> >> > *-*-riscv) libstdcxx_atomic_lock_policy=mutex ;;
> >> > *-*-*) AC_TRY_COMPILE([ ... ],,[],[])
> >> > esac
> >> >
> >> > but this way is simpler. If we add more customization for other
> >> > targets we can reconsider using the 'case "${host}"' form.
> >>
> >> Ya, that's kind of where I came to as well -- the proper autoconf flavor
> >> would scale way better, but hopefully nobody else makes this mistake and
> >> thus we don't need to worry about that.
> >
> > <nod>
> >
> >> I'm fine with either way (though I think we'd need a "riscv*" there, to
> >> match riscv32 and riscv64?), so if you want to swap it over (or have me
> >> re-spin this) it's no big deal on my end -- also fine, as per below,
> >> with you just committing this ;)
> >
> > Yeah, I figured *-*-riscv probably wasn't right, so that's another
> > reason to prefer your approach.
> >
> >
> >>
> >> > So this is OK for trunk, modulo regenerating libstdc++-v3/configure
> >> > with this change. Let me know if you want me to do that regen for you
> >> > (or commit the whole thing for you).
> >>
> >> That'd be great, thanks! It usually takes me a while to get all the
> >> autotools versions lined up (we just got new machines at the office),
> >> that way I won't have to do so.
> >
> > No problem, I can regen+push for you.
>
> Great, thanks!
Pushed as r12-8161-g3fc22eedb033cb
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-14 16:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-07 18:46 [PATCH v1] libstdc++: Default to mutex-based atomics on RISC-V Palmer Dabbelt
2022-04-14 15:08 ` Jonathan Wakely
2022-04-14 15:18 ` Palmer Dabbelt
2022-04-14 15:22 ` Jonathan Wakely
2022-04-14 15:24 ` Palmer Dabbelt
2022-04-14 16:39 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).