[PATCH] D30787: [builtins] Implement emulated TLS on Windows.

[Frederich Munch via Phabricator via llvm-commits]

marsupial marked an inline comment as done.
marsupial added inline comments.


================
Comment at: lib/builtins/emutls.c:204
+
+/* When built with MSVC or clang without compiler-rt then the following needs
+ * to be provided.
----------------
hans wrote:
> It's not about compiler-rt; Clang defines __ATOMIC_ACQUIRE et al. as built-in macros: https://clang.llvm.org/docs/LanguageExtensions.html#c11-atomic-builtins
> 
> Same thing with the __atomic_load_n and store_n functions.
> 
> GCC also provides these macros and functions. Probably it's better to just guard on `#if !defined(__ATOMIC_RELEASE)`.
> 
> But shouldn't you also define the macros in a compatible way? I.e. shouldn't __ATOMIC_ACQUIRE be 2 and _RELEASE be 3? With your enum they'll be 0 and 1.
I'm not sure matching the enum values is that important as each function is used once.
But updated them as I was already changing the ifdef & comment.


https://reviews.llvm.org/D30787