[Openmp-commits] [PATCH] D137168: [OpenMP][mingw] Fix build for aarch64 target

[Martin Storsjö via Phabricator via Openmp-commits]

mstorsjo added inline comments.


================
Comment at: openmp/runtime/src/kmp_os.h:621
 
+#elif KMP_ARCH_AARCH64 && defined(__GNUC__)
+
----------------
natgla wrote:
> mstorsjo wrote:
> > natgla wrote:
> > > mstorsjo wrote:
> > > > natgla wrote:
> > > > > For MSVC we compiled libomp only with MSVC, so the patch is a bit different. @branh will start upstreaming the changes later this month, they will be complimentary to this change. 
> > > > > Am I right that this patch will fix the issues only if compiled with gcc?
> > > > > looks good to me.
> > > > This patch fixes issues if compiled with clang in mingw mode - gcc doesn't support windows on aarch64 (yet).
> > > Is this: "defined(__ GNUC __)" for mingw then? 
> > Yes, that's the correct compiler define to look for; clang identifies itself both as `__GNUC__` and `__clang__` on most platforms (except for MSVC targets) - it does that for mingw targets too.
> For clang most of this file is using KMP_COMPILER_CLANG
Sure - but that would limit the new codepaths to clang only, right? The used `__sync_*` intrinsics are GCC intrinsics that Clang also supports. If someone was to build this code for Windows on ARM64 with a future GCC that supports this target, this would be the right codepath to use in that case too (there's nothing clang-specific that GCC can't handle here).

Then again I don't know if this openmp implementation expects to support GCC overall at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137168