[PATCH] D36071: [compiler-rt] [builtins] Only include immintrin.h if it is used

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D36071#828513, @marsupial wrote:

> I'm wondering if the **#ifdef** is now a dead code path entirely.
>  Do you have any insight into how ARM is getting **__atomic_load_n** and x86/64 not?


I had only tested it with clang for ARM, where the whole ifdef isn't used. I hadn't tested this with MSVC for ARM though (I only use compiler-rt when building with clang-mingw, not when building with MSVC), but it does indeed fail there. The _load/store_be_u32/64 intrinsics aren't perhaps the most canonical way of doing atomic loads/stores, but MSVC is pretty badly lacking in that department. They do document this though:

https://msdn.microsoft.com/en-us/library/windows/desktop/ms684122(v=vs.85).aspx

> Simple reads and writes to properly-aligned 32-bit variables are atomic operations. In other words, you will not end up with only one portion of the variable updated; all bits are updated in an atomic fashion. However, access is not guaranteed to be synchronized. If two threads are reading and writing from the same variable, you cannot determine if one thread will perform its read operation before the other performs its write operation.



> Simple reads and writes to properly aligned 64-bit variables are atomic on 64-bit Windows. Reads and writes to 64-bit values are not guaranteed to be atomic on 32-bit Windows. Reads and writes to variables of other sizes are not guaranteed to be atomic on any platform.

Although I'm not sure how well that applies to non-x86 platforms. I can update it to use the Interlocked* intrinsics that are available for many platforms. They're a bit hairy though since not all of them are available for all platforms, and it varies a little across MSVC versions as well.


https://reviews.llvm.org/D36071