Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch onnxruntime to compile with clang-cl #165

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

kokulshan
Copy link
Contributor

Changes

Add some patches to the onnxruntime port to handle issues with llvm-rc and clang-cl related to unicode in the rc file and clang-cl needing simd flags enabled when compiling SIMD code.
...

Triplet Support

  • x64-windows (MSVC and Clang-Cl)

@luncliff luncliff self-assigned this Feb 3, 2024
@luncliff luncliff added the platform: windows Windows, UWP label Feb 3, 2024
@luncliff
Copy link
Owner

luncliff commented Feb 3, 2024

Appreciate your work, @kokulshan .
I was watching microsoft/vcpkg#31028 and clang-cl support will be followed in the registry after it.

I will test the update soon. 🏃

@luncliff luncliff self-requested a review February 3, 2024 03:04
* several features fail with clang-cl, but ok
* may requires customized triplet file for clang-cl
@luncliff
Copy link
Owner

luncliff commented Feb 3, 2024

@kokulshan Would you comment your triplet content?
Future visitors may be helped with it.

This is what I used

set(VCPKG_TARGET_ARCHITECTURE x64)
set(VCPKG_CRT_LINKAGE dynamic)
set(VCPKG_LIBRARY_LINKAGE dynamic)

# check https://github.com/microsoft/vcpkg/pull/31028
set(VCPKG_ENV_PASSTHROUGH_UNTRACKED "VCPKG_ROOT")
find_file(VCPKG_CHAINLOAD_TOOLCHAIN_FILE NAMES "scripts/toolchains/clangcl.cmake" HINTS "$ENV{VCPKG_ROOT}" NO_DEFAULT_PATH REQUIRED)

set(VCPKG_LOAD_VCVARS_ENV ON)
# For ports which uses Visual Studio. Not for Ninja
# set(VCPKG_PLATFORM_TOOLSET "ClangCL")

if(VCPKG_TARGET_ARCHITECTURE STREQUAL "x86")
    set(VCPKG_CXX_FLAGS "-m32")
elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "x64")
    set(VCPKG_CXX_FLAGS "-m64")
elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm")
    set(VCPKG_CXX_FLAGS "--target=arm-pc-windows-msvc")
elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64")
    set(VCPKG_CXX_FLAGS "--target=arm64-pc-windows-msvc")
endif()
# prevent CMake to insert semicolon
set(VCPKG_CXX_FLAGS "-fms-compatibility ${VCPKG_CXX_FLAGS}")
set(VCPKG_C_FLAGS   ${VCPKG_CXX_FLAGS})

In the clangcl.cmake,

# ...
find_program(CMAKE_CXX_COMPILER NAMES clang-cl REQUIRED)
find_program(CMAKE_C_COMPILER NAMES clang-cl REQUIRED)
find_program(CMAKE_AR NAMES llvm-lib REQUIRED)
find_program(CMAKE_RC NAMES llvm-rc REQUIRED)
# find_program(CMAKE_LINKER NAMES lld-link REQUIRED)
# ...

@luncliff luncliff merged commit 4591b57 into luncliff:main Feb 5, 2024
10 of 12 checks passed
@kokulshan
Copy link
Contributor Author

kokulshan commented Feb 6, 2024

@kokulshan Would you comment your triplet content? Future visitors may be helped with it.

I cannot share my triplet or toolchain file due to it having a bunch of work specific stuff in there. But it is very similar to your example toolchain file. Two differences are that I explicilty set the C and CXX flags for clang to /ARCH:AVX2 for our code and we also set the linker to lld-link

@luncliff
Copy link
Owner

luncliff commented Feb 6, 2024

That's OK. Feel free to suggest new PR if you meet another errors 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: windows Windows, UWP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants