Improve definition of CAMLthread_local for C++ compatibility#147
Draft
Improve definition of CAMLthread_local for C++ compatibility#147
CAMLthread_local for C++ compatibility#147Conversation
51c444b to
ab47aa3
Compare
CAMLthread_local for C++ compatibilityCAMLthread_local for C⁠+⁠+ compatibility
CAMLthread_local for C⁠+⁠+ compatibilityCAMLthread_local for C++ compatibility
f9f7c20 to
3483c77
Compare
bd8f4bd to
dbf630c
Compare
dbf630c to
336f66a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since OCaml 5, with multicore support, the runtime is written in C11 and uses thread-local storage. There's only one variable (a POD) that is public in the OCaml headers. It was first using GCC's
__thread, then we changed it to usethread_localin C23 and C++11, and_Thread_localin C111, for portability. We hit the ABI incompatibility problem that is described in N2850:_Thread_localfor better C++ interoperability with C, as if a C++ compiler reads our header, it will use the C++thread_localABI to access the variable, instead of the C11_Thread_localsemantics.We hit problems on Cygwin, and I'm expecting problems with MSVC. See
#14220and#13541.Looking to fix this problem once and for all, I've found N2850, submitted to both the C and C++ working groups. There is no definitive fix, the paper suggests adding
_Thread_localto C++, to make it easier to write common headers, or to haveextern "C" thread_local(in C++) behave like C11 semantics for the thread-local ABI instead of C++'s. The paper also suggests moving away from C23thread_localspelling, to avoid confusion with C++11thread_local, and remain with C11_Thread_local.I contacted the authors of the paper for advice, here's the short answer:
Missing a standard way to use C semantics within a C++ compiler, we have to resort to compiler extensions. The compiler extensions (
__thread,__declspec(thread)) seem to always be equivalent to_Thread_local. Here are my proposed changes:_Thread_localinternally, and for variables protected byCAML_INTERNALS. My preference is to use the standard keyword wherever possible.For exposed symbols (currently only the
caml_state):If using MSVC, prefer
__declspec(thread).In C++, also use
[[msvc::no_tls_guard]]2 on the declaration to avoid intermediary functions checking if the variable has been initialized. Unfortunately Clang doesn't support this attribute (https://github.com/llvm/llvm-project/issues/57696), so for the*-pc-windowstarget in C++ we have to fallback to the__threadkeyword.Otherwise the compiler is GCC or Clang, or another compiler (nowadays Intel® oneAPI DPC++/C++ Compiler and IBM Open XL C/C++ for AIX are both based on LLVM), and supports
__threadwith GCC's semantics:As an extension, Clang-based compilers also support the C
_Thread_localkeyword in C++ (GCC doesn't), but we can use__threadinstead. Unfortunately MSVC rejects mixing__declspec(thread)and_Thread_localin declarations and definitions.To help convincing, here's a complete example: https://godbolt.org/z/TKxd1nbMY, with the output of GCC (4.9.4 and trunk), MSVC (19.38 introduced C11 atomic), and Clang (trunk, targets
x86_64-pc-windowandx86_64-*-linux), running both in C and C++, showing that the generated assembly is identical. If you want to come up with a different macro dance, you may validate it with this example.Footnotes
_Thread_localis a C11 keyword. The macrothread_local, defined in<threads.h>, expands to_Thread_local. If__STDC_NO_THREADS__3 is defined to1, the header is not provided. In C23,thread_localbecomes a keyword, and the spelling_Thread_localis discouraged. ↩Support for the attribute can be tested with
__has_cpp_attribute(standard in C++20), supported by all MSVC and Clang version that we're interested in. However for an attribute with a namespace we need to check first for__cplusplusas GCC until 11 didn't support the namespace syntax in C mode. ↩There was a bug where Microsoft implemented C11
<threads.h>, forgot to ship the header, and did not define the guard macro. ↩