Remove -latomic from -pthread spec

[r-value]

https://github.com/riscv-collab/riscv-gcc/blob/ca312387ab141060c20c388d83d6fc4b2099af1d/gcc/config/riscv/linux.h#L43

Is it really necessary to link against libatomic for pthread to function correctly? Is there any direct relation like a symbol reference?

I doubt it because since glibc 2.34, libpthread is removed as a separate library and I found that a program which only utilize pthread_*() or std::thread actually works without -pthread, i.e. it works without libatomic.

Also, according to gcc/libstdc++/using concurrency, the use of atomic should be checked if it needs to be linked against libatomic. If there is no direct relation between the two libs, we'd better not link against libatomic when -pthread is present. The checks and link operations should be done/generated by user at build system level on user's discretion.

[aswaterman]

In practice, it fixes the build for a large number of packages. People love to write programs with _Atomic char etc. and then link them with -pthread, which on RISC-V fails to link.

I can understand your point, but it would create massive headaches.

[aswaterman]

BTW, I know some people have discussed open-coding/inlining the subword atomics so that libatomic is no longer needed. We can obviously make your proposed change after that work is performed.

[r-value]

Actually this workaround itself is causing headaches now.

Since -pthread is not required to compile these programs, CMake FindThreads module won't add -pthread flag at all. This actually exposed the problem again, making this workaround more or less pointless now. IMO it's better to accept the fact that we need to add libatomic detections downstream and drop this, instead of keeping this confusing behavior.

[r-value]

Also, we can't just force FindThreads to use -pthread if not required because some of the linkers actually doesn't accept it, interpreting it as -p -t hread.

[r-value]

This confusing behavior is also upsetting our efforts to port packages to RISC-V architecture in the correct way of adding detections for libatomic because of this confusion between pthread and atomic. Some of the upstream authors mistakenly believe that linking against atomic should be -pthread's work, despite the fact that it's not documented and semantically incorrect.

[r-value]

I know some people have discussed open-coding/inlining the subword atomics so that libatomic is no longer needed.

This is no doubt the best solution. But as for now, the problem is spreading along with the popularity of glibc 2.34. Removal of this confusing behavior will be a strong support for us to convince upstream authors to accept our porting patch.

[XieJiSS]

FYI, this can be a possible example of headache: telegramdesktop/tdesktop#24275

[XieJiSS]

Also note that CMake reverted their workaround (see: Kitware/CMake@c6da90b) on this because "the -pthread flag can pass on compilers like XL, that interprets it as -p -t hread and returns zero". Seems like adding a patch to CMake is more difficult than we thought, as they must maintain compatibility and consider many corner cases.

[aswaterman]

@XieJiSS’s example hits home.

I’m not the decision-maker here, but I think changing this behavior without inlining the atomics would be the worst of both worlds: we would end up with two broken universes, instead of just one. To make forward progress, it would be best to accelerate the effort to open-code the atomics.

[r-value]

https://github.com/riscv-collab/riscv-gcc/blob/2c4857d0981501b7c50bbf228de9e287611f8ae5/gcc/config/riscv/linux.h#L31

I'm curious about why this line is replaced by " %{pthread:" LD_AS_NEEDED_OPTION " -latomic " LD_NO_AS_NEEDED_OPTION "}"

This commit is from #12. Why someone changed the spec to only link against libatomic when -pthread is present? Why not just link against it by default?

[davidlt]

IIRC this is because pthread needs subword atomic support, and in general pthread is used in a lot of projects. This solved a large number of issues (but not all). This is probably the most annoying thing in RISC-V as this tends to break packages randomly.

There is patch for GCC from Rivos Inc. (Mar 11, 2022): [PATCH v2] RISCV: Add support for inlining subword atomics
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281336.html

See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104338

There is probably another 3-4 GCC bugzilla ticket over the last several years about this (or similar issues).

[XieJiSS]

Hi David, I also think that the reason they added --as-needed -latomic is to support some functions of libpthread, but my test showed that actually pthread works fine without -latomic: https://shz.al/Bw8Z (compiled with gcc 11.2.0, glibc 2.34). The same code can compile with gcc 11.1.0 and glibc 2.33 if -lpthread (not -pthread to avoid internal -latomic) is present.

I already declared sum as char, guessing that modifying subword values with pthread mutex will result in calls to libatomic, but seems like I'm wrong. Note that this test code also uses the outdated gcc builtin __sync_add_and_fetch, which (to my surprise) works fine without -latomic, mostly because I've read before that __atomic builtins are proposed to replace __sync stuffs but the newer __atomic_add_fetch_1 requires -latomic.

Currently, I'm struggling to find a piece of test code which uses only pthread functions, without making any progress. I also reviewed some projects in which I used to fix the atomic_xxx linking error with -pthread, and noticed that actually they all use atomic in some sort of ways, e.g. std::atomic<bool> or __atomic_exchange_n(bool_value, ...).

David, do you have further information on this (like the code that made the gcc guys believe libatomic is necessary for libpthread)? This will help a lot, as we might be able to use that to persuade upstream util/toolchain projects like CMake to update their pthread detection code. Thanks!

[XieJiSS]

There is patch for GCC from Rivos Inc. (Mar 11, 2022): [PATCH v2] RISCV: Add support for inlining subword atomics https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281336.html

This is definitely one of the best news I've heard about in this year :-D

[r-value]

There is patch for GCC from Rivos Inc. (Mar 11, 2022): [PATCH v2] RISCV: Add support for inlining subword atomics

We may still need some workaround for existed projects since the patches are not released yet 🤔 Also, the patch was sent too late, glibc 2.34+ is already causing problems.
But I'm really excited that we can see the finish line of libatomic fixes. FINALLY! 🎉

[CoelacanthusHex]

There is patch for GCC from Rivos Inc. (Mar 11, 2022): [PATCH v2] RISCV: Add support for inlining subword atomics https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281336.html

This is really good and exciting, the rest will be left to time.
When the downstream uses this version, our task will be completed.
🎉🎉🎉

[XieJiSS]

There is patch for GCC from Rivos Inc. (Mar 11, 2022)

Oh, I just realized that maybe they should change values of related macros like ATOMIC_BOOL_LOCK_FREE and ATOMIC_CHAR_LOCK_FREE. The value of std::atomic<bool>::is_always_lock_free and std::atomic_is_lock_free might also need to be updated. See also: #15 (comment)

I'll send a email to the mailing list regarding this, but I think it might be better to leave a comment here for reference.

[aswaterman]

All of these are lock-free, both under the current regime and the proposed new one. They’re all LR/SC-based, regardless, and such sequences don’t constitute locking.

[XieJiSS]

Yes, I agree that both implementation are lock-free, but according to #15:

But atomic_always_lock_free is intended to be used in cases where we require a compile-time constant, so it returns true if we can directly emit an atomic sequence, otherwise it returns false. It doesn't matter that libatomic might be able to emit the sequence.

So it is returning false for bool and char and other subword values, and IMO this should be changed in the patch.

[andreas-schwab]

There is patch for GCC from Rivos Inc. (Mar 11, 2022): [PATCH v2] RISCV: Add support for inlining subword atomics https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281336.html

Note that the patch has support for atomic fetch only so far. Atomic store and exchange are still missing, without them it doesn't actually solve the libatomic problem yet.

[r-value]

I think linking against libatomic as needed by default for all situation is quite a reasonable option. It was the workaround for #12, but I just couldn't understand why someone limited this behavior to -pthread just days after that.
The commit history is squashed so I can't find why the latter change was made.

[r-value]

Also, the spec change is a lot simpler than opem-coding, which is more likely to be released earlier. I think we can change the spec while waiting for the development of the patch.

[davidlt]

FYI
This is part of agenda for [RISCV] RISC-V GNU Toolchain Biweekly Sync-up call (Aripl 07, 2022)
https://gcc.gnu.org/pipermail/gcc/2022-April/238501.html

It's too late to make such change to GCC 12 (which probably will be release within weeks). Thus the most likely candidate would be the next GCC version in 2023.

[palmer-dabbelt]

This is best talked about on the GCC mailing lists and bugzilla. There's some ABI fallout from changing the defaults we need to sort out, it should all be mentioned in the bugs already.