There's a handful of bugs sort of related to this one, but nothing specific. This has been a long-standing issue and I think folks are generally familiar with it, but just to summarize: we don't have sub-word atomic instruction in RISC-V, so we just call out to the libatomic routines. This causes fallout in a handful of places (see 86005 and 81358, for example) and there's been some attempts to resolve it but nothing appears to have stuck. I figured it'd be a good starter project for Patrick, as he's yet to do any GCC stuff. He's working through it and doesn't have anything to post yet, but figured I'd just open the bug now so folks knew what was going on from our end.
Bug 89835 comment #2 references this.
This also causes a lot of packages to fail to build with undefined references to the libatomic libcalls.
Created attachment 52368 [details] Patch v1 Patch submitted to mailing list. Subject: [PATCH v1] RISC-V: Add support for inlining subword atomic operations.
In short term, maybe we can change the spec to link against libatomic by default (implemented in https://github.com/riscv-collab/riscv-gcc/commit/2c4857d0981501b7c50bbf228de9e287611f8ae5). It will solve a lot of build errors if we revert the value of `LIB_SPEC` instead of only link against libatomic when `-pthread` is present. Detailed talk about this: https://github.com/riscv-collab/riscv-gcc/issues/337
(In reply to rvalue from comment #4) > In short term, maybe we can change the spec to link against libatomic by > default (implemented in > https://github.com/riscv-collab/riscv-gcc/commit/ > 2c4857d0981501b7c50bbf228de9e287611f8ae5). It will solve a lot of build > errors if we revert the value of `LIB_SPEC` instead of only link against > libatomic when `-pthread` is present. > > Detailed talk about this: > https://github.com/riscv-collab/riscv-gcc/issues/337 We talked through some options like that and decided it was too risky for GCC-12. We already found one ABI break related to this (see 84568), and want to make sure we give distros adequate advance notice before something that we know to break ABIs. That said, it's really not a GCC ABI break, it's a per-package configure issue. We can fix the libstdcxx fallout, which is the only bit we know about right now (though it's not like we've scrubbed builds for this). If the folks building distros think it's better to risk the ABI breaks rather than chase around the build failures, then I'm fine rushing something in to GCC-12. I see Andreas is already here, I'm having some trouble adding anyone else though (I can never quite figure out Bugzilla...).
Kito pointed out earlier today that it should already be possible to default to libatomic via a --with-specs=... configure-time argument already, so one option here would be to just add an example/reference spec to GCC. That would allow distros to opt in to the "always link libatomic" behavior, if they want to risk the ABI-related issues like we see in libstdcxx (which we'd of course have to fix). It doesn't sort out the long-tail issues related to ABI compatibility between GCC and LLVM (and the suggested mappings), but at least it gives folks a unified mechanism for doing this. I know it's pretty late, but that seems like something we could do on the GCC-12 timeline. It seems like the distro folks are pretty fed up with waiting so they're just going to backport/hack this if we miss GCC-12, might as well have one way for that to happen.
FYI Patch v2 has been submitted in the mailing list, see https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281336.html I'm not sure why bugzilla lost sync with the mailing list.
Created attachment 52835 [details] Patch v3 Patch submitted to mailing list. Subject: [PATCH v3] RISC-V: Add support for inlining subword atomic operations.
(In reply to RZ Pan (XieJiSS) from comment #7) > I'm not sure why bugzilla lost sync with the mailing list. There is no sync between gcc-patches and bugzilla, and never has been.
I am not subscribed to the mailing list, but I am not able to see any more mails after the v3. Any update?
*** Bug 108564 has been marked as a duplicate of this bug. ***
I've got a somewhat recently rebased version of Patrick's patch floating around, it passed testing but I got hung up on the futex_time64 thing and forgot about it. Not sure if folks think it's too late for the upcoming CGC release, but I wouldn't be opposed to taking it -- looks like distros aro going to apply workarounds if we don't do something, so at least this way there'll be a single workaround in trunk. There's some bigger fixes in the works for the whole memory model as we've got other issues, but since those are a bit tricker it might be worth just doing the stop-gap thing for now.
I see v3 of the patch at https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593378.html And v4 at https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604584.html with last reply at https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606366.html
I picked this back up, v7 is here: https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616080.html
The master branch has been updated by Patrick O'Neill <poneill@gcc.gnu.org>: https://gcc.gnu.org/g:f797260adaf52bee0ec0e16190bbefbe1bfc3692 commit r14-269-gf797260adaf52bee0ec0e16190bbefbe1bfc3692 Author: Patrick O'Neill <patrick@rivosinc.com> Date: Tue Apr 18 14:33:13 2023 -0700 RISCV: Inline subword atomic ops RISC-V has no support for subword atomic operations; code currently generates libatomic library calls. This patch changes the default behavior to inline subword atomic calls (using the same logic as the existing library call). Behavior can be specified using the -minline-atomics and -mno-inline-atomics command line flags. gcc/libgcc/config/riscv/atomic.c has the same logic implemented in asm. This will need to stay for backwards compatibility and the -mno-inline-atomics flag. 2023-04-18 Patrick O'Neill <patrick@rivosinc.com> gcc/ChangeLog: PR target/104338 * config/riscv/riscv-protos.h: Add helper function stubs. * config/riscv/riscv.cc: Add helper functions for subword masking. * config/riscv/riscv.opt: Add command-line flag. * config/riscv/sync.md: Add masking logic and inline asm for fetch_and_op, fetch_and_nand, CAS, and exchange ops. * doc/invoke.texi: Add blurb regarding command-line flag. libgcc/ChangeLog: PR target/104338 * config/riscv/atomic.c: Add reference to duplicate logic. gcc/testsuite/ChangeLog: PR target/104338 * gcc.target/riscv/inline-atomics-1.c: New test. * gcc.target/riscv/inline-atomics-2.c: New test. * gcc.target/riscv/inline-atomics-3.c: New test. * gcc.target/riscv/inline-atomics-4.c: New test. * gcc.target/riscv/inline-atomics-5.c: New test. * gcc.target/riscv/inline-atomics-6.c: New test. * gcc.target/riscv/inline-atomics-7.c: New test. * gcc.target/riscv/inline-atomics-8.c: New test. Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
The releases/gcc-13 branch has been updated by Patrick O'Neill <poneill@gcc.gnu.org>: https://gcc.gnu.org/g:55088cf384d4c43e280ec794217e32fab64070ba commit r13-7336-g55088cf384d4c43e280ec794217e32fab64070ba Author: Patrick O'Neill <patrick@rivosinc.com> Date: Tue Apr 18 14:33:13 2023 -0700 RISCV: Inline subword atomic ops RISC-V has no support for subword atomic operations; code currently generates libatomic library calls. This patch changes the default behavior to inline subword atomic calls (using the same logic as the existing library call). Behavior can be specified using the -minline-atomics and -mno-inline-atomics command line flags. gcc/libgcc/config/riscv/atomic.c has the same logic implemented in asm. This will need to stay for backwards compatibility and the -mno-inline-atomics flag. 2023-05-03 Patrick O'Neill <patrick@rivosinc.com> gcc/ChangeLog: PR target/104338 * config/riscv/riscv-protos.h: Add helper function stubs. * config/riscv/riscv.cc: Add helper functions for subword masking. * config/riscv/riscv.opt: Add command-line flags -minline-atomics and -mno-inline-atomics. * config/riscv/sync.md: Add masking logic and inline asm for fetch_and_op, fetch_and_nand, CAS, and exchange ops. * doc/invoke.texi: Add blurb regarding new command-line flags -minline-atomics and -mno-inline-atomics. libgcc/ChangeLog: PR target/104338 * config/riscv/atomic.c: Add reference to duplicate logic. gcc/testsuite/ChangeLog: PR target/104338 * gcc.target/riscv/inline-atomics-1.c: New test. * gcc.target/riscv/inline-atomics-2.c: New test. * gcc.target/riscv/inline-atomics-3.c: New test. * gcc.target/riscv/inline-atomics-4.c: New test. * gcc.target/riscv/inline-atomics-5.c: New test. * gcc.target/riscv/inline-atomics-6.c: New test. * gcc.target/riscv/inline-atomics-7.c: New test. * gcc.target/riscv/inline-atomics-8.c: New test. Signed-off-by: Patrick O'Neill <patrick@rivosinc.com> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
Closing this issue since the fix is on trunk and was backported to releases/gcc-13 to be included in the next release.
Thanks Patrick for working on that and for the backport to the gcc-13 branch. I wonder if the following patch should also be backported, as it doesn't make sense to link with -latomic anymore with inline subword atomic operations: commit 203f3060dd363361b172f7295f42bb6bf5ac0b3b Author: Andreas Schwab <schwab@suse.de> Date: Sat Apr 23 15:48:42 2022 +0200 riscv/linux: Don't add -latomic with -pthread Now that we have support for inline subword atomic operations, it is no longer necessary to link against libatomic. This also fixes testsuite failures because the framework does not properly set up the linker flags for finding libatomic. The use of atomic operations is also independent of the use of libpthread. gcc/ * config/riscv/linux.h (LIB_SPEC): Don't redefine.
(In reply to Aurelien Jarno from comment #18) > I wonder if the following patch should also be backported, as it > doesn't make sense to link with -latomic anymore with inline subword atomic > operations Agreed. It's now meaningless to keep this workaround for RISC-V as the problem has been resolved.
(In reply to rvalue from comment #19) > (In reply to Aurelien Jarno from comment #18) > > I wonder if the following patch should also be backported, as it > > doesn't make sense to link with -latomic anymore with inline subword atomic > > operations > > Agreed. It's now meaningless to keep this workaround for RISC-V as the > problem has been resolved. Yep. Can someone send the backport?