[Openmp-commits] [PATCH] D53422: Support clang compiling under windows-gnu and windows-msvc

[Jonathan Peyton via Phabricator via Openmp-commits]

jlpeyton requested changes to this revision.
jlpeyton added a comment.
This revision now requires changes to proceed.

Add two lines in `kmp_config.h.cmake`:

  #cmakedefine01 MSVC
  #define KMP_MSVC_COMPAT MSVC

And instead of having the `!defined(__MINGW32__)` or `#ifndef __MINGW32__` guard, use `KMP_MSVC_COMPAT` instead.



================
Comment at: runtime/cmake/config-ix.cmake:85-87
+  if(CMAKE_C_COMPILER_ID STREQUAL "Clang")
+    check_c_compiler_flag(-mrtm LIBOMP_HAVE_MRTM_FLAG)
+  endif()
----------------
I don't think the `if(CMAKE_C_COMPILER_ID STREQUAL "Clang")` condition is necessary here.  


================
Comment at: runtime/src/kmp_affinity.h:378-397
+      for (size_t i = 0; i < static_cast<size_t>(__kmp_num_proc_groups); ++i)
         mask[i] = 0;
     }
     void copy(const KMPAffinity::Mask *src) override {
       const Mask *convert = static_cast<const Mask *>(src);
-      for (size_t i = 0; i < __kmp_num_proc_groups; ++i)
+      for (size_t i = 0; i < static_cast<size_t>(__kmp_num_proc_groups); ++i)
         mask[i] = convert->mask[i];
----------------
Change all `size_t i` to `int i` instead of adding the `static_cast<size_t>`


================
Comment at: runtime/src/kmp_dispatch.cpp:27
 #include "kmp_str.h"
-#if KMP_OS_WINDOWS && KMP_ARCH_X86
+#if KMP_OS_WINDOWS && KMP_ARCH_X86 && !KMP_COMPILER_CLANG
 #include <float.h>
----------------
It would be nice if you could add another macro to `kmp_os.h` like this:
```
#define KMP_USE_X87CONTROL (KMP_OS_WINDOWS && KMP_ARCH_X86 && !KMP_COMPILER_CLANG)
```

and use it here and below in this file.

What was the problem with clang-cl?  Why did it fail with _control87()?


================
Comment at: runtime/src/kmp_os.h:249
 
-#if __GNUC__ >= 4
+#if __GNUC__ >= 4 && !defined(__MINGW32__)
 #define __forceinline __inline
----------------
This use of `__MINGW32__` is ok.


================
Comment at: runtime/src/kmp_os.h:299
 
-#if KMP_OS_WINDOWS
+#if KMP_OS_WINDOWS && !defined(__GNUC__)
 #define KMP_ALIGN(bytes) __declspec(align(bytes))
----------------
Will using `KMP_MSVC_COMPAT` work here as well instead of `__GNUC__`?


================
Comment at: runtime/src/kmp_settings.cpp:5516-5520
           int num_bits_in_mask = 0;
           for (int bit = init_mask->begin(); bit != init_mask->end();
                bit = init_mask->next(bit))
             num_bits_in_mask++;
+          exactly_one_group = (num_bits_in_group == static_cast<DWORD>(num_bits_in_mask));
----------------
Change `num_bits_in_mask` from `int` to `DWORD` instead of `static_cast<>`


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D53422