Feature/win arm64 support - #3718
Conversation
…test.bat and vars_win.bat
…-aashik-mcw/oneDAL into feature/win-arm64-support
|
@nithin-aashik-mcw - thanks - might be we would go straight to bazel build system? |
|
@napetrov Just to clarify, would you prefer that Windows ARM64 support be implemented in the Bazel build system instead? |
icfaust
left a comment
There was a problem hiding this comment.
I'm hoping someone with more build knowledge with me especially on the llvm/windows side of things can tell our status on that currently on x86, while this adds good CI/testing conformance for llvm+arm+win, the macros are not coded for arm, but for llvm+win.
| TBB_VERSION: v2023.1.0 | ||
| OPENBLAS_VERSION: v0.3.33 |
There was a problem hiding this comment.
Need to be added to renovate.json for proper tracking.
| pull_request: | ||
| branches: | ||
| - main | ||
| - 'feature/**' |
There was a problem hiding this comment.
Help me understand the addition of feature/**
| - name: Setup MSYS2 | ||
| shell: pwsh | ||
| run: | | ||
| Invoke-WebRequest https://github.com/msys2/msys2-installer/releases/download/2026-06-11/msys2-${{ matrix.os.arch }}-20260611.exe -UseBasicParsing -OutFile msys2-${{ matrix.os.arch }}.exe | ||
| .\msys2-${{ matrix.os.arch }}.exe install --confirm-command --root "C:\msys64" |
There was a problem hiding this comment.
Msys2 provides a github action for this, I think we can trust them on that and would be lower maintenance (unless not available for this os/cpu combo?) let me know
| - name: Install LLVM for Win-${{ matrix.os.arch }} | ||
| shell: pwsh | ||
| run: | | ||
| Invoke-WebRequest https://github.com/llvm/llvm-project/releases/download/llvmorg-22.1.8/LLVM-22.1.8-${{ matrix.os.llvm_arch }}.exe -UseBasicParsing -OutFile LLVM-${{ matrix.os.llvm_arch }}.exe |
There was a problem hiding this comment.
This way of installing llvm is likely to be high maintenance (hardcoded version in a url) is there any way to make this better for renovate tracking?
| Start-Process msiexec.exe -ArgumentList "/i cmake-${{ matrix.os.arch }}.msi /quiet /norestart" -Wait | ||
| echo "C:\Program Files\CMake\bin" >> $env:GITHUB_PATH | ||
|
|
||
| Invoke-WebRequest https://github.com/ninja-build/ninja/releases/download/v1.13.2/ninja-win${{ matrix.os.arch }}.zip -OutFile ninja-win${{ matrix.os.arch }}.zip |
There was a problem hiding this comment.
Help explain the use of ninja
| #ifdef __GNUC__ | ||
| #define DAAL_DEPRECATED __attribute__((deprecated)) | ||
| #elif defined(_MSC_VER) | ||
| #elif defined(_MSC_VER) && !defined(__clang__) |
There was a problem hiding this comment.
This should impact normal llvm builds on x86 using windows. Is this something we have checked what impact this may have?
| #elif defined(TARGET_ARM) && defined(_MSC_VER) | ||
| #define KUSER_SHARED_DATA_ADDR 0x7FFE0000UL | ||
| #define PROCESSOR_FEATURES_OFFSET 0x274 | ||
| #define PF_ARM_SVE_INSTRUCTIONS_AVAILABLE 46 |
There was a problem hiding this comment.
This probably needs some code comments to make it more maintainable.
| $$(if $$(CHECK_DLL_SIG),Wintrust.lib)) | ||
| $$(eval daaldep.winarm.rt.dpc := $$(dep_dpc) \ | ||
| $$(if $$(CHECK_DLL_SIG),Wintrust.lib)) | ||
| $$(eval daaldep.win.threxport.create = grep -v -E '^(;|$$$$$$$$)' $$$$< $$$$(USECPUS.out.grep.filter)) |
| @@ -0,0 +1,84 @@ | |||
| @echo off | |||
| rem ============================================================================ | |||
| rem Copyright 2020 Intel Corporation | |||
There was a problem hiding this comment.
If this is not a copy-paste, please use the new copyright header:
https://github.com/uxlfoundation/oneDAL/blob/main/CONTRIBUTING.md#copyright-guidelines-for-contributions
|
|
||
| setlocal | ||
|
|
||
| rem req: PowerShell 3.0+ |
There was a problem hiding this comment.
Aren't batch files meant to be runnable with the old Cmd.exe alone? Feels quite odd to launch powershell from a .bat file.
| powershell.exe -command "if ($PSVersionTable.PSVersion.Major -lt 3) {Write-Host \"The script requires PowerShell 3.0 or above (current version: $($PSVersionTable.PSVersion.Major).$($PSVersionTable.PSVersion.Minor))\"; exit 1} else {exit 0}" | ||
| if errorlevel 1 goto Error_load | ||
|
|
||
| if /I not "%PROCESSOR_ARCHITECTURE%"=="ARM64" ( |
There was a problem hiding this comment.
Does this mean that cross-compilation is not supported?
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup MSYS2 |
There was a problem hiding this comment.
Would it not be a problem to mix msys2 with LLVM-based tools? And does msys2 provide something that the LLVM stack doesn't?
|
|
||
| /* oneDAL 64-bit integer types */ | ||
| #if !(defined(__INTEL_COMPILER) || defined(__INTEL_LLVM_COMPILER)) && defined(_MSC_VER) | ||
| #if !(defined(__INTEL_COMPILER) || defined(__INTEL_LLVM_COMPILER)) && defined(_MSC_VER) && !defined(__clang__) |
There was a problem hiding this comment.
I understand the CI is using clang-cl, not plain clang. Isn't that compiler supposed to provide the same headers with these kinds of definitions?
| os: [ | ||
| { name: GitHub hosted, label: windows-11-vs2026-arm, cxx: clang, arch: arm64, isa: sve, llvm_arch: woa64 } | ||
| ] | ||
| name: Windows-${{ matrix.os.arch }}-${{ matrix.os.cxx }}-${{ matrix.os.isa }}-${{ matrix.os.name }} |
There was a problem hiding this comment.
Overall comment: would it be possible to build with conda-forge tooling as a separate job?
https://conda-forge.org/blog/2026/02/09/win-arm64/
| static void * serv_malloc(size_t size, size_t alignment) { return aligned_alloc(alignment, size); } | ||
| static void * serv_malloc(size_t size, size_t alignment) { | ||
| #if defined(_MSC_VER) | ||
| return _aligned_malloc(size, alignment); |
There was a problem hiding this comment.
aligned_alloc is part of the C++17 standard:
https://en.cppreference.com/cpp/memory/c/aligned_alloc
Does MSVC or clang-cl not offer it?
| #define PRAGMA_OMP_SIMD PRAGMA_TO_STR(omp simd) | ||
| #define PRAGMA_OMP_SIMD_ARGS(ARGS) PRAGMA_TO_STR_(omp simd ARGS) | ||
| #elif defined(_MSC_VER) | ||
| #elif defined(_MSC_VER) && !defined(__clang__) |
There was a problem hiding this comment.
This is in a series of elses that already checked for clang. It would have no effect.
| -DDAAL_REF -DONEDAL_REF -DDAAL_CPU=sve -Werror -Wno-empty-body -Wreturn-type \ | ||
| $(COMPILER.lnx.clang.target) \ | ||
| $(COMPILER.sysroot) | ||
| COMPILER.win.clang= clang++ -march=armv8-a+sve \ |
There was a problem hiding this comment.
As far as I know, there is no requirement to support SVE in order to run windows on ARM.
Add Windows ARM64 (winarm) build support
Adds native build support for oneDAL on Windows ARM64 using clang/lld-link, extending the existing
win32e(MSVC) andlnxarmtargets.Build system
winarmplatform target with clang/lld-link/llvm-lib toolchain (winarm.mk,clang.ref.arm.mk,arm.mk,lnxarm.mk,vc.mkl.32e.mk,common.mk)winarmin top-levelPLATs; made Windows flags (-Fo,-Q,-EHsc, link rules) conditional on MSVC vs. clang$(_IA)) instead of hardcoded tointel64, in bothmakefileanddeps.ref.mkvcvarsall.bat(replacing x64-onlyvcvars64.bat)Core / algorithm compatibility (clang-cl ≠ real MSVC)
_MSC_VER-only code paths with!defined(__clang__)acrossdaal_defines.h,service_defines.h,service_topo.cpp,env_detect_features.cpp, KD-tree KNN kernel, andoneapi/dal/{backend,detail}/common.hppKUSER_SHARED_DATA_aligned_malloc/_aligned_freeon Windows instead of POSIXaligned_alloc/freeCI / tooling
build.bat/test.bat: detect host arch (AMD64/ARM64), setPLAT=winarm, propagateARCH_DIRthrough TBB/OpenBLAS pathstbb.batdeleting source dirs after moveOpenBlas-*→OpenBLAS-*)deploy/local/vars_win.bat: deriveDAAL_IAfrom host arch instead of hardcodingintel64examples/cmake/setup_examples.cmake/samples/cmake/setup_samples.cmake: recognizeARM64, fixintel_intel64→intel64namingTesting
feature/win-arm64-support