[HIPIFY][Feature] Ninja support
Problem Description
Using the Ninja CMake generator (https://ninja-build.org/), HIPIFY fails to build with MSVC.
I'm following the instructions at https://github.com/ROCm/HIPIFY/blob/amd-staging/docs/building/building-hipify.rst, swapping -G "Visual Studio 17 2022" for -G Ninja. I regularly use Ninja for other projects with MSVC on Windows, including LLVM, without issues (it's a much nicer experience than the Visual Studio generator IME).
The build fails due to compilation commands passing MSVC flags to the clang++ compiler after this code changes the value of CMAKE_CXX_COMPILER from (for example) C:/Program Files (x86)/Microsoft Visual Studio/2022/BuildTools/VC/Tools/MSVC/14.42.34433/bin/Hostx64/x64/cl.exe to clang++: https://github.com/ROCm/HIPIFY/blob/f601f964840ba61b064d1ab20a533d57f2a8cd7a/CMakeLists.txt#L90-L93
Sample error logs:
[1/39] Building CXX object CMakeFiles\hipify-clang.dir\src\CUDA2HIP.cpp.obj
FAILED: CMakeFiles/hipify-clang.dir/src/CUDA2HIP.cpp.obj
D:\projects\llvm-project\dist\bin\clang++ /nologo /TP -DCLANG_BUILD_STATIC -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_HAS_EXCEPTIONS=0 -ID:\projects\llvm-project\dist\include /DWIN32 /D_WINDOWS /GR /EHsc -DLIB_CLANG_RES=21 /O2 /Ob2 /DNDEBUG -MD -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -DUNICODE -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS /EHs-c- /GR- /std:c++17 /Od /GR- /EHs- /EHc- /MP /Zc:preprocessor /showIncludes /FoCMakeFiles\hipify-clang.dir\src\CUDA2HIP.cpp.obj /FdCMakeFiles\hipify-clang.dir\ /FS -c D:\projects\TheRock\sources\HIPIFY\src\CUDA2HIP.cpp
clang++: error: no such file or directory: '/nologo'
clang++: error: no such file or directory: '/TP'
clang++: error: no such file or directory: '/DWIN32'
clang++: error: no such file or directory: '/D_WINDOWS'
Full error logs: https://gist.github.com/ScottTodd/10cca27eba121216129b2111f2e316b1
Operating System
Windows 11 (10.0.22631)
CPU
AMD Ryzen Threadripper PRO 7975WX 32-Cores
ROCm Version
ROCm 6.3.1
ROCm Component
HIPIFY
Steps to Reproduce
-
Build clang from LLVM source:
git clone [email protected]:llvm/llvm-project.git cd llvm-project cmake -G Ninja -B build ./llvm \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_PREFIX=dist \ -DLLVM_TARGETS_TO_BUILD="" \ -DLLVM_ENABLE_PROJECTS="clang" \ -DLLVM_INCLUDE_TESTS=OFF cmake --build build --target install -
Build HIPIFY:
git clone [email protected]:ROCm/HIPIFY.git cd HIPIFY cmake -G Ninja -B build-ninja . \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_PREFIX_PATH="D:\projects\llvm-project\dist" \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DHIPIFY_INSTALL_CLANG_HEADERS=OFF cmake --build build -
See errors: https://gist.github.com/ScottTodd/10cca27eba121216129b2111f2e316b1
(Optional for Linux users) Output of /opt/rocm/bin/rocminfo --support
No response
Additional Information
Building using the Visual Studio generator does work successfully. Swap step 2 in my reproducer with
cmake -B build-vs . \
-DCMAKE_BUILD_TYPE=Release \
-DHIPIFY_INSTALL_CLANG_HEADERS=OFF \
-DCMAKE_PREFIX_PATH="D:\projects\llvm-project\dist" \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON
cmake --build build-vs --config Release
https://gist.github.com/ScottTodd/a29c712370fa7b83fd64931675be8965
Changing this code to keep the existing CMAKE_CXX_COMPILER fixes the build for me: https://github.com/ROCm/HIPIFY/blob/f601f964840ba61b064d1ab20a533d57f2a8cd7a/CMakeLists.txt#L90-L93
I'm not sure if there is something mandatory about using the compiler from LLVM_TOOLS_BINARY_DIR though. I would suspect that any build toolchain could be used, so long as the linking step against clang/LLVM libraries succeeds.
We're starting to build many ROCm projects as part of https://github.com/nod-ai/TheRock. Other projects so far have been compatible with Ninja.
I see these solutions for TheRock:
- Try switching to the Visual Studio generator (perhaps for only some subprojects like HIPIFY)
- Carry a local patch working around this issue (as I'm doing in https://github.com/nod-ai/TheRock/pull/104)
- We land a fix here ("upstream" from TheRock's perspective)
This is the patch I've created in TheRock to workaround this for now: https://github.com/nod-ai/TheRock/blob/main/patches/amd-mainline/HIPIFY/0003-Don-t-override-CMAKE_C-XX-_COMPILER-on-MSVC.patch
diff --git a/CMakeLists.txt b/CMakeLists.txt
index dad79610..f3616fbc 100755
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -115,6 +115,10 @@ if (NOT HIPIFY_CLANG_TESTS_ONLY)
${LLVM_BINARY_DIR}/tools/lld/include
${LLVM_EXTERNAL_LLD_SOURCE_DIR}/include)
endif()
+ elseif(MSVC)
+ # Keep the existing compiler since MSVC-specific flags will be set later.
+ # Overriding CMAKE_CXX_COMPILER/CMAKE_C_COMPILER below is sketchy - could
+ # that be removed or reworked instead?
else()
set(CMAKE_CXX_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang++)
set(CMAKE_C_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang)
--
2.47.1.windows.2
Hello, @ScottTodd. I think we need Ninja support not only for MSVS but Linux/GNU, too.
Looking at how some other ROCm projects are set up, overriding CMAKE_CXX_COMPILER could be done via toolchain files. For example:
- https://github.com/ROCm/rocPRIM/blob/develop/toolchain-linux.cmake
- https://github.com/ROCm/rocPRIM/blob/develop/toolchain-windows.cmake
It seems like HIPIFY builds just fine on my systems using MSVC though, so requiring a toolchain might be unneeded complexity. That being said, HIPIFY already requires clang anyways.
Hi @ScottTodd,
HIPIFY already requires clang anyways
It is not quite so. hipify-clang doesn't require clang in its work, and it might be built by MSVC or GNU C++, besides clang, as you have already seen for yourself. But the clang compiler toolchain is not needed during the hipify-clang's work, as all the needed clang/LLVM libraries are statistically linked. What is needed for hipify-clang to hipify CUDA sources is clang's distribution header files.
Anyway, what about Ninja support for Linux builds? I think it might look strange, if we support it only for Windows/MSVS.