[PATCH] D36071: [builtins] Use _Interlocked* intrinsics for atomics on MSVC

[Frederich Munch via Phabricator via llvm-commits]

marsupial added inline comments.


================
Comment at: lib/builtins/emutls.c:219
 #else
-    return (uintptr_t) _load_be_u32(ptr);
+    return _InterlockedAdd64(ptr, 0);
+#endif
----------------
mstorsjo wrote:
> marsupial wrote:
> > mstorsjo wrote:
> > > X64 doesn't have `Add64`, but have got `Or64_np`. ARM (and according to MSVC 2017's `intrin.h`, ARM64 as well) have got `Add64` though.
> > The docs for these functions say it returns the original value.
> > Should be fine as 0 is added/ored, but a comment explaining the situation may be wise.
> Sure, I can add that. Does my approach look sensible otherwise? Do you think I should add inline comments about the gotchas in different versions as motivation for each of them?
Overall seems proper/better. I did a quick check, and the previous load/store ops are significantly faster than the interlocked variants (which is possible why I chose them), but think it adds too much complexity and weirdness for expanding it to other architectures.

So just a simple comment about the or/add of zero is returning the original value that makes the operation equivalent to a pure load is enough for me.

I think the **#ifdefs** are enough/self-explanatory as to platform availability.




https://reviews.llvm.org/D36071