[Openmp-commits] [PATCH] D138737: [openmp] Use GCC style intrinsics for atomics on Clang-cl on aarch64 too

[Martin Storsjö via Phabricator via Openmp-commits]

mstorsjo added a comment.

In D138737#3952929 <https://reviews.llvm.org/D138737#3952929>, @DavidSpickett wrote:

> Can you explain the structure of the existing #ifdef? I'm having trouble following it.
>
> I think the idea is that gcc on AArch64 would end up in the following `#elif (KMP_ASM_INTRINS && KMP_OS_UNIX) || !(KMP_ARCH_X86 || KMP_ARCH_X86_64)`. On Windows, you'd fall down into the `#else`. But what each of those blocks includes I'm not sure.

As far as I can see, the high level structure right now is this:

  #if KMP_ASM_INTRINS && KMP_OS_WINDOWS && !((KMP_ARCH_AARCH64 || KMP_ARCH_ARM) && defined(__GNUC__))
  
  
  // MSVC Intrinsics
  // Used on MSVC on all archs, plus used by Clang on Windows on x86
  
  #if KMP_ARCH_AARCH64 && KMP_COMPILER_MSVC && !KMP_COMPILER_CLANG
  
  // MSVC/AArch64 specific bits
  
  #else // !KMP_ARCH_AARCH64
  
  // MSVC/X86 specifics
  
  #endif
  
  
  #elif (KMP_ASM_INTRINS && KMP_OS_UNIX) || !(KMP_ARCH_X86 || KMP_ARCH_X86_64)
  
  // GCC Intrinsics, __sync_*, __atomic_*
  
  #else
  
  // Relying on external function definitions
  
  #endif

Clang hasn't implemented the full set of MSVC intrinsics for atomics, so for Windows on non-x86, Clang is better off with the GCC style intrinsics of `__sync_*` and `__atomic_*`. D137168 <https://reviews.llvm.org/D137168> / a356782426f5bf54a00570e1f925345e5fda7b2e <https://reviews.llvm.org/rGa356782426f5bf54a00570e1f925345e5fda7b2e> added `&& !(KMP_ARCH_AARCH64 && defined(__GNUC__))` to the first condition, to single out the aarch64+mingw condition, but the same issue (Clang doesn't handle the full set of MSVC aarch64 atomic intrinsics) is present in Clang in MSVC/clang-cl mode too. (D138689 <https://reviews.llvm.org/D138689> / 97958c9bb83cec44b6ce13e732b53de0171a5d43 <https://reviews.llvm.org/rG97958c9bb83cec44b6ce13e732b53de0171a5d43> then added ARM to the same condition.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138737/new/

https://reviews.llvm.org/D138737