Clang compilation of CUDA backend on Windows #3345

j8asic · 2020-09-06T21:12:02Z

As clang-cl is does not enable CUDA compilation (/clang:--cuda*** options do not work), the code changes here enable clang++ to compile Kokkos. So one can simply use CMake with ninja and build the project. Most of the errors were due to clashing of preprocessor defines (see also #3344).
Changes:

remove addition of -x cu for Clang, as it already uses -x cuda
avoid Windows atomics, use GNU's coming with Clang
use __thread instead of __declspec(thread), see here
extend _atomic_compare_exchange_strong to accept sizeof(T) == 16 (needed when using a pair of int64 as arg)
include Windows.h for GetCurrentProcessId in Kokkos_Core.cpp

dalg24-jenkins · 2020-09-06T21:12:04Z

Can one of the admins verify this patch?

crtrott · 2020-09-07T22:49:46Z

core/src/impl/Kokkos_Core.cpp

@@ -55,6 +55,8 @@
 #include <cerrno>
 #ifndef _WIN32
 #include <unistd.h>
+#else
+#include <Windows.h>


Is this needed because msvc includes it by default but msvc-cl doesn't?

Needed because clang-cl also includes it, but pure clang++ doesn't.

crtrott · 2020-09-07T22:50:11Z

core/src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp

@@ -381,7 +381,7 @@ KOKKOS_INTERNAL_INLINE_DEVICE_IF_CUDA_ARCH bool _atomic_compare_exchange_strong(
    T* dest, T compare, T val, MemoryOrderSuccess, MemoryOrderFailure,
    typename std::enable_if<
        (sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 ||
-         sizeof(T) == 8) &&
+         sizeof(T) == 8 || sizeof(T) == 16) &&


Does this work also with msvc?

Wow seem it doesn't. This was my attempt of a clean fix for #3344.
This is the msvc (clang-cl) error:

2>C:\Code\kokkos\core\src\impl/Kokkos_Atomic_Generic.hpp(210,16): error : no matching function for call to 'atomic_compare_exchange' 2>C:\Code\kokkos\core\src\impl/Kokkos_Atomic_Generic.hpp(556,16): message : in instantiation of function template specialization 'Kokkos::Impl::atomic_fetch_oper<Kokkos::Impl::AddOper<long long, const long long>, long long>' requested here 2>C:\Code\kokkos\core\src\impl/Kokkos_Atomic_Increment.hpp(132,11): message : in instantiation of function template specialization 'Kokkos::atomic_fetch_add<long long>' requested here 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(154,12): message : candidate function not viable: no known conversion from 'unsigned long long *' to 'volatile int *const' for 1st argument 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(162,13): message : candidate function not viable: no known conversion from 'unsigned long long *' to 'volatile long *const' for 1st argument 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(174,21): message : candidate function not viable: no known conversion from 'unsigned long long *' to 'volatile unsigned int *const' for 1st argument 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(180,22): message : candidate function not viable: no known conversion from 'unsigned long long *' to 'volatile unsigned long *const' for 1st argument 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(189,10): message : candidate template ignored: requirement 'sizeof(unsigned long long) == sizeof(int)' was not satisfied [with T = unsigned long long] 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(208,10): message : candidate template ignored: requirement 'sizeof(unsigned long long) != sizeof(int) && sizeof(unsigned long long) == sizeof(long)' was not satisfied [with T = unsigned long long] 2>C:\Code\kokkos\core\src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp(253,10): message : candidate template ignored: requirement '(sizeof(unsigned long long) != 4) && (sizeof(unsigned long long) != 8)' was not satisfied [with T = unsigned long long]

If you're okay with this - I'll implement a define to distinguish between clang++ and clang-cl, because msvc gets activated when _MSC_VER is found.

#if defined(_MSC_VER) && !defined(KOKKOS_COMPILER_INTEL) #define KOKKOS_COMPILER_MSVC _MSC_VER #endif

Tried to fix this using c9535e4, see below.

crtrott · 2020-09-07T22:50:56Z

OK to test.

crtrott · 2020-09-07T22:51:16Z

This looks generally good. I will test on my machine too on windows. Thanks for this!

masterleinad · 2020-09-08T00:30:47Z

You will also need to fix the indentation using clang-format 8 or the patch

diff --git a/core/src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp b/core/src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp
index c4685df..a652c24 100644
--- a/core/src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp
+++ b/core/src/impl/Kokkos_Atomic_Compare_Exchange_Strong.hpp
@@ -380,8 +380,8 @@ template <class T, class MemoryOrderSuccess, class MemoryOrderFailure>
 KOKKOS_INTERNAL_INLINE_DEVICE_IF_CUDA_ARCH bool _atomic_compare_exchange_strong(
     T* dest, T compare, T val, MemoryOrderSuccess, MemoryOrderFailure,
     typename std::enable_if<
-        (sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 ||
-         sizeof(T) == 8 || sizeof(T) == 16) &&
+        (sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 || sizeof(T) == 8 ||
+         sizeof(T) == 16) &&
             std::is_same<
                 typename MemoryOrderSuccess::memory_order,
                 typename std::remove_cv<MemoryOrderSuccess>::type>::value &&

crtrott · 2020-09-08T02:22:09Z

We can apply the format patch if you like, and its clang 8.0.0 (yeah it is that specific for clang-format ...).

j8asic · 2020-09-08T05:38:15Z

We can apply the format patch if you like, and its clang 8.0.0 (yeah it is that specific for clang-format ...).

Sure, please do.

This looks generally good. I will test on my machine too on windows. Thanks for this!

I was in doubt (and forgot to push) a change to include --cuda-path flag, which I'll do asap. What's the more appropriate variable to use for this: CUDAToolkit_BIN_DIR or Kokkos_CUDA_DIR?

j8asic · 2020-09-08T20:49:36Z

As the new commit summary writes, it solves the preprocessor clashing between Windows, CUDA & GNU atomics. Btw KOKKOS_COMPILER_CLANG_MSVC is added to CMake (you mentioned in my last pull request it should be added). This solves #3344, but #3346 is still there (since const dim3 grid code mixes unsigned and signed types for min function).

dalg24 · 2020-09-09T12:47:03Z

Please look at dalg24:j8asic_windows_clang_cuda

I applied clang-format on the two commits and edited the author name from "adminfesb <adminfesb@JEFE.local>" -> "j8asic <j8asic@gmail.com>" in the first commit.

I tried to push directly to your branch but the permissions changed and it was rejected.

j8asic · 2020-09-09T17:39:44Z

@dalg24 I have no idea what happened (long time haven't used Windows)...
Ok, I've just set-up clang-format and for future I'll style it...

dalg24 · 2020-09-09T18:23:23Z

@dalg24 I have no idea what happened (long time haven't used Windows)...
Ok, I've just set-up clang-format and for future I'll style it...

Either grant me permission to push to your branch and I will fix it or force push my branch to yours.

dalg24 · 2020-09-10T03:38:25Z

Retest this please

dalg24 · 2020-09-16T04:07:11Z

Retest this please

cmake/kokkos_enable_devices.cmake

crtrott

Some issues see comments.

crtrott · 2020-09-16T04:10:51Z

cmake/kokkos_enable_devices.cmake

@@ -25,6 +25,16 @@ IF (KOKKOS_ENABLE_PTHREAD)
  SET(KOKKOS_ENABLE_THREADS ON)
 ENDIF()

+# as CMAKE_CXX_SIMULATE_ID does not work, detect clang-cl to avoid clang++,
+# cl, and clang-cl clashes, requires CMake >= 3.15


This doesn't work we have lower cmake version as minimum

I had this in mind. For a lower-version cmake, it will default to clang-cl. Users that specifically use clang++ need CMake >= 3.15.

crtrott · 2020-09-16T04:12:09Z

cmake/kokkos_enable_devices.cmake

-    #link omp library from LLVM lib dir
+  IF(KOKKOS_COMPILER_CLANG_MSVC)
+    #for clang-cl expression /openmp yields an error, so directly add the specific Clang flag
+    SET(ClangOpenMPFlag /clang:-fopenmp=libomp)


This got added in the Travis build now, and thus fails.

Ok I see the error, will change it. Thanks.

* add --cuda-path clang option * add KOKKOS_COMPILER_CLANG_MSVC if using clang-cl * Clang CUDA compilation needs GNU atomics activated * fixed clashing of Windows and GNU atomic functions declarations

crtrott · 2020-10-28T03:23:41Z

I fixed the CUDA path issue in CMake on a branch which also rebases on top of #3532: (my branch is on crtrott/windows-clang-with-cuda). But it turns out that compared to 3532 this broke the nvcc windows build.

crtrott

This breaks the existing nvcc build on windows. Question: do you want me to force push a rebase to this branch, or shall I open a new PR with the rebase of this branch on #3532 and then we continue there?

crtrott · 2020-10-28T03:40:14Z

I think I figured it out. https://github.com/crtrott/kokkos/tree/windows-clang-with-cuda

crtrott · 2020-10-28T17:43:02Z

Also @j8asic can you check my branch and see whether your build still works as expected? All our builds now work (nvcc on linux, clang for cuda on linux, nvcc in VS, msvc in VS, clang-cl in VS).

crtrott · 2020-10-29T02:08:33Z

I force pushed my rebase onto this.

…nto windows-clang-with-cuda

j8asic · 2020-10-29T17:01:52Z

Checked out https://github.com/crtrott/kokkos/tree/windows-clang-with-cuda

First error: generally under Windows:

CMake Error at C:/Code/kokkos-crtrott/build/KokkosConfig.cmake:55 (INCLUDE):
  INCLUDE could not find load file: C:/Code/kokkos-crtrott/build/KokkosTargets.cmake

I usually comment the include line to continue...

Second error, SET_TARGET_PROPERTIES in KokkosConfig.cmake sets linking dirs without quotes, which due to spaces fails (CUDA is by default installed in Program Files). INTERFACE_INCLUDE_DIRECTORIES and INTERFACE_LINK_LIBRARIES should be with quotes.
Otherwise, GNU clang++ compiles nicely.
The sad thing is that I get runtime errors on reductions (illegal memory access) and an error when using constant-space kernels (cudaErrorInvalidDeviceFunction). Local-space (lightweight hint) with parallel_for works. I'll open an issue for this and try to see why is this happening.
In CMake when setting nvcc as the compiler, I get the following output:

Setting default Kokkos CXX standard to 14
The CXX compiler identification is MSVC 19.27.29112.0
Detecting CXX compiler ABI info
Detecting CXX compiler ABI info - failed
Check for working CXX compiler: C:/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe
Check for working CXX compiler: C:/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe - broken
CMake Error at C:/Program Files/CMake/share/cmake-3.19/Modules/CMakeTestCXXCompiler.cmake:59 (message):
  The C++ compiler

    "C:/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe"
  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: C:/Code/kokkos-crtrott/build/CMakeFiles/CMakeTmp
    
    Run Build Command(s):C:/Code/ninja.exe cmTC_972e6 && [1/2] Building CXX object CMakeFiles\cmTC_972e6.dir\testCXXCompiler.cxx.obj

    FAILED: CMakeFiles/cmTC_972e6.dir/testCXXCompiler.cxx.obj 

    C:\PROGRA~1\NVIDIA~2\CUDA\v10.1\bin\nvcc.exe  /nologo /TP   /DWIN32 /D_WINDOWS /W3 /GR /EHsc  /MDd /Zi /Ob0 /Od /RTC1 /showIncludes /FoCMakeFiles\cmTC_972e6.dir\testCXXCompiler.cxx.obj /FdCMakeFiles\cmTC_972e6.dir\ /FS -c testCXXCompiler.cxx

    nvcc fatal   : Don't know what to do with 'C:/FS'
    ninja: build stopped: subcommand failed.

Don't know why nvcc is detected as MSVC...

crtrott · 2020-11-02T17:31:24Z

Hi, what do you mean with "general in Windows" the appveyor test on Windows did pass, so at least a simple MSVC build seems to work. Regarding NVCC: you need some setup fancy. See here: #3063 (comment) Note you have to update all the paths according to your install. And those paths change every time you update Visual Studio ...

j8asic · 2020-11-02T19:40:14Z

My ninja setup is the same as yours. I had this nvcc problem like another user, since googling brought me to Kokkos issues. The culprit in my case is PATH containing both CUDA dir and MSVC compiler dir, CMake then forces that nvcc is detected as cl, don't know why. In any case, I removed the MSVC path and it works. So nvcc + cl works for me as well, run some test also, confirmed.

Second point is that KokkosTargets.cmake shows up on installing the built lib. It an inconvenience as I would maybe want to use CMake build dir as Kokkos dir directly, but okay...

KokkosConfig.cmake still gives errors as it should use quotes when setting Windows paths (lines 42, 43, 49, 50). KokkosConfigCommon.cmake line 105 as well.

Finally, clang++ works like confirmed above, but I'm frustrated that I get these runtime errors with functors in const-space memory and reductions... As said, I'm going to look into it...

Do you want me to fix CMake errors and push it to my branch?

crtrott · 2020-11-03T02:11:37Z

Yeah try pushing the fixes to cmake and we can recheck if everything works.

crtrott · 2020-11-03T02:12:21Z

Note: the travis failure is probably an unrelated timeout.

crtrott · 2020-11-05T23:29:43Z

@j8asic if you still want this in for the next release we need to do this soon. We are starting release process next week.

crtrott · 2020-11-06T16:09:27Z

Thanks @j8asic . I will check this again with our configs, and if it all passes we can have that in the next release

j8asic · 2020-11-06T16:16:01Z

@crtrott you're welcome. It works, but for example I'm not sure what happens when TPL_INCLUDES is a list.
KOKKOS_APPEND_CONFIG_LINE("INTERFACE_INCLUDE_DIRECTORIES \"${TPL_INCLUDES}\"")
Because then each item should be quoted, and KOKKOS_APPEND_CONFIG_LINE is not friendly about the following:
KOKKOS_APPEND_CONFIG_LINE(INTERFACE_INCLUDE_DIRECTORIES "${TPL_INCLUDES}")

Btw I'm not fixed to a release, and am completely demotivated, since I wasted a month trying to deploy on Windows and lost users:

nvcc 11 + msvc cannot handle relevant c++17 code (some compiler bugs in deep_copy and variadic args unrolling) :(
clang with cuda has runtime issue due to who knows what :(

crtrott · 2020-11-10T00:14:51Z

oh man that is a bummer. These compiler issues are really a drain on all of us.

crtrott · 2020-11-10T16:10:07Z

Passed with MSVC, NVCC+MSVC and MSVC-Clang on my machine.

masterleinad

Looks reasonable.

j8asic mentioned this pull request Sep 7, 2020

min/max problems on Windows #3346

Closed

crtrott reviewed Sep 7, 2020

View reviewed changes

j8asic mentioned this pull request Sep 9, 2020

atomic_compare_exchange undefined for pair_int_t type #3344

Closed

dalg24 reviewed Sep 16, 2020

View reviewed changes

cmake/kokkos_enable_devices.cmake Outdated Show resolved Hide resolved

crtrott requested changes Sep 16, 2020

View reviewed changes

adminfesb and others added 9 commits October 27, 2020 14:50

clang++ compiles CUDA backend on Windows

bae8451

Update clang++ and clang-cl detection and preprocessor for atomics

241d1d4

* add --cuda-path clang option * add KOKKOS_COMPILER_CLANG_MSVC if using clang-cl * Clang CUDA compilation needs GNU atomics activated * fixed clashing of Windows and GNU atomic functions declarations

clang++ compiles CUDA backend on Windows

59f13d1

Update clang++ and clang-cl detection and preprocessor for atomics

91a3179

* add --cuda-path clang option * add KOKKOS_COMPILER_CLANG_MSVC if using clang-cl * Clang CUDA compilation needs GNU atomics activated * fixed clashing of Windows and GNU atomic functions declarations

KOKKOS_COMPILER_CLANG_MSVC detection fix

1d02640

Fix format

833ff18

Fix format 2nd attempt

dc33633

O M G

73a4caf

Fix CMake CUDAPath detection

5052b1d

crtrott requested changes Oct 28, 2020

View reviewed changes

Fix NVCC based windows build again.

1e5f9d6

crtrott added 2 commits October 28, 2020 19:13

More formating for all of us.

4036285

Merge branch 'windows-clang-with-cuda' of github.com:crtrott/kokkos i…

7afe161

…nto windows-clang-with-cuda

TPL options have quotes around paths for Windows

86184fb

crtrott approved these changes Nov 10, 2020

View reviewed changes

masterleinad approved these changes Nov 10, 2020

View reviewed changes

DavidPoliakoff approved these changes Nov 10, 2020

View reviewed changes

crtrott merged commit 29fb7fb into kokkos:develop Nov 10, 2020

masterleinad mentioned this pull request Feb 28, 2023

Windows Support #1533

Closed

Clang compilation of CUDA backend on Windows #3345

Clang compilation of CUDA backend on Windows #3345

Conversation

j8asic commented Sep 6, 2020

dalg24-jenkins commented Sep 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott Sep 7, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott commented Sep 7, 2020

crtrott commented Sep 7, 2020 • edited

masterleinad commented Sep 8, 2020

crtrott commented Sep 8, 2020 • edited

j8asic commented Sep 8, 2020

j8asic commented Sep 8, 2020

dalg24 commented Sep 9, 2020 • edited

j8asic commented Sep 9, 2020

dalg24 commented Sep 9, 2020

dalg24 commented Sep 10, 2020

dalg24 commented Sep 16, 2020

crtrott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott commented Oct 28, 2020

crtrott left a comment

Choose a reason for hiding this comment

crtrott commented Oct 28, 2020

crtrott commented Oct 28, 2020

crtrott commented Oct 29, 2020

j8asic commented Oct 29, 2020

crtrott commented Nov 2, 2020

j8asic commented Nov 2, 2020

crtrott commented Nov 3, 2020

crtrott commented Nov 3, 2020

crtrott commented Nov 5, 2020

crtrott commented Nov 6, 2020

j8asic commented Nov 6, 2020

crtrott commented Nov 10, 2020

crtrott commented Nov 10, 2020

masterleinad left a comment

Choose a reason for hiding this comment

crtrott Sep 7, 2020 •

edited

crtrott commented Sep 7, 2020 •

edited

crtrott commented Sep 8, 2020 •

edited

dalg24 commented Sep 9, 2020 •

edited