-
-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide atomic helper functions #384
Comments
Looking at the usual llvm-only Unix suspects I see:
Related finds:
Conclusion: I don't know, both seem fine. |
On modern platforms (at least the last decade), it seems like macOS is shipping the
Well it sounds like they have something like a
It's not so much about what others do; larger OS vendors that have these bits integrated as a proper shared system library don't need to configure it via the regular compiler-rt build system. It mostly boils down to this comment at https://reviews.llvm.org/D71600#1868502:
This comment is from 2020; the standalone libatomic mode was added in 2021 here: llvm/llvm-project@d56729b But in any case; for us it only boils down to whether we consider atomic access (to uncommonly large variables) across DLLs something we care about supporting properly - is it worth the extra inconvenience of an extra runtime DLL and a bit more build system mess? As very few projects use this - presumably none of the ones within MSYS2 clang64 today as this is missing there right now AFAIK - almost nobody would be inconvenienced by the extra runtime DLL anyway. But it does make things a little bit more complicated in the build, e.g. 5087d5d. The only user I'm aware of right now, is libcxx's testsuite. |
ok, I mostly care that build systems/projects with unix in mind just work ideally, which is why I looked there. From what I understand both ways should work fine in that regard. I don't have a strong opinion otherwise. |
I am not familiar with the atomic helper functions, but looking at it briefly it seems I was curious and looked up what MSVC
|
I guess there’s such a risk, in theory, but if that would be the case, I think the libc++ build would be failing now already. As we don’t have these symbols at all right now, any currently working build can’t be using them (unless they explicitly test for their availability in a configure test). |
|
OpenBSD uses their own build system and does the equivalent of COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF. |
Thanks! I presume it's within a shared library in the end? |
FreeBSD and OpenBSD only ship a PIC static library for libcompiler_rt. https://github.com/freebsd/freebsd-src/blob/main/lib/libcompiler_rt/Makefile |
Oh, interesting. I guess that they don't consider cross-library atomic access (for unusually sized atomic variables that need the helpers) an issue then. On the other hand, as long as one doesn't link shared libraries with Anyway, thanks for the data point, I guess this counts as a vague +1 for the case of " |
With the LLVM 18.x release candidates coming up soon, I think I'll go ahead and merge the approach of using |
The latest (pre)release, at https://github.com/mstorsjo/llvm-mingw/releases/tag/20240130, should have atomics helpers now. |
It was noted in llvm/llvm-project#73398 (comment) that we're lacking atomic helper functions.
These functions are used as fallback, when the compiler can't generate inline code for atomic access to variables. In practice, it seems like calls to these functions are generated when using variables larger than 8 bytes, marked as atomic.
When this happens, Clang also prints a warning like this:
In GCC, these helpers are provided in
libatomic-1.dll
, and if using them, you need to link with-latomic
. (Callers that expect to use them might want to try to see if it is possible to link with-latomic
, and if possible do that, otherwise leave it out.)For LLVM based toolchains, compiler-rt builtins don't provide these helpers by default, but they can. But we'd need to settle on how to do that.
It can be done in two different ways; we can set
-DCOMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=FALSE
(this defaults toTRUE
) when building the compiler-rt builtins. Then we get these helper functions as part of the regular compiler-rt builtins library, which we link automatically, so everything works, without needing to add-latomic
, and no extra runtime DLL is needed.There's one drawback with this approach: The atomic helper functions use locks for making the accesses actually atomic. But if we statically link the locks into each separate DLL/EXE, then each module would use their own locks, so accesses to atomic variables larger than 8 bytes across DLLs would end up not properly atomic. Practically speaking, this is a very rare case though (the case in libcxx was the first time I hear someone request these helper functions at all), and I guess it is even more rare to have such atomics in the public DLL interface.
The alternative is to build compiler-rt builtins with
-DCOMPILER_RT_BUILD_STANDALONE_LIBATOMIC=TRUE
. This produces a newlibclang_rt.atomic_dynamic-<arch>.dll
and corresponding.dll.a
. (Building it requires a recent fix, llvm/llvm-project@76b8975.) It is possible to copylibclang_rt.atomic_dynamic-<arch>.dll.a
and name itlibatomic.dll.a
, so that users that check for-latomic
will succeed and find it, even if the actual linked DLL has a different name.I've tried implementing both of these; see master...atomics and master...atomics-standalone.
CCing @lazka @MehdiChinoune @mati865 @jeremyd2019 @alvinhochun @cjacek for some opinions on the matter before settling which way to go here. Do you think we should go for the complex solution with the separate DLL, or just keep it simple for now? (As user projects tend to need to check for
-latomic
anyway, we can probably easily switch to that form later, if we feel we need to.)The text was updated successfully, but these errors were encountered: