[HIPIFY][Feature] Ninja support

[ScottTodd]

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++:

HIPIFY/CMakeLists.txt

Lines 90 to 93 in f601f96

	else()
	set(CMAKE_CXX_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang++)
	set(CMAKE_C_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang)
	endif()

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

1. 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

2. 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

3. 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:

HIPIFY/CMakeLists.txt

Lines 90 to 93 in f601f96

	else()
	set(CMAKE_CXX_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang++)
	set(CMAKE_C_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang)
	endif()

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 Enable HIPIFY on Windows. nod-ai/TheRock#104)
• We land a fix here ("upstream" from TheRock's perspective)

[ScottTodd]

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

[emankov]

Hello, @ScottTodd. I think we need Ninja support not only for MSVS but Linux/GNU, too.

[ScottTodd]

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.

[emankov]

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.