Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Windows ARM64 support to TorchSharp by implementing platform detection, native library loading improvements, and build system modifications to handle ARM64 architecture on Windows.
- Updates runtime identification to detect win-arm64 platform
- Adds explicit dependency chain loading for Windows native libraries
- Includes Windows ARM64-specific LibTorch redistributables and build configurations
Reviewed Changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/TorchSharp/Torch.cs | Updates platform detection and native library loading for ARM64 |
| src/Redist/libtorch-cpu/*.sha | Adds SHA hashes for ARM64 LibTorch redistributables |
| src/Redist/libtorch-cpu/libtorch-cpu.proj | Defines ARM64-specific DLL dependencies |
| src/Native/gen-buildsys-win.bat | Adds ARM64 CMake configuration support |
| src/Native/build.cmd | Enables ARM64 build architecture and removes conditional vcvarsall calls |
| src/Examples.Utils/Examples.Utils.csproj | Updates ImageSharp package version |
| pkg/* | Adds ARM64 NuGet package configuration and references |
| Directory.Build.* | Configures ARM64 build properties and native assembly references |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } else { | ||
| ok = TryLoadNativeLibraryByName("torch_cpu", typeof(torch).Assembly, trace); | ||
| ok = TryLoadNativeLibraryByName("LibTorchSharp", typeof(torch).Assembly, trace); | ||
| var isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); |
There was a problem hiding this comment.
The variable isWindows is redeclared here when it's already available from line 129. Consider reusing the existing variable to avoid duplication.
@dotnet-policy-service agree |
|
Hey @cm4ker, huge thanks for the contribution! Have you had a chance to check that these changes still work on other platforms? Is Windows x64 still all good? Our pipelines don’t yet support Windows arm64, but we’re working on upgrading them so we can support the newer releases we've been working on (moving to the latest libtorch + CUDA) |
|
@alinpahontu2912 Hi! Unfortunately I haven't x64 device and can't check it =( |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Hey @cm4ker thanks a lot for the contribution ! We would also need to be able to build and create the nuggets in our pipelines and then figure out how to properly distribute them. Not yet sure if we can have arm64 and x64 nuggets simultaneously. I promise I will look into it, as it seems there's some interest in the topic and I've also worked on porting pytorch for windows arm64 so this holds a soft spot for me. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // On Windows, also attempt direct absolute-path loads from the assembly directory | ||
| if (!ok && isWindows) { | ||
| var asmDir = Path.GetDirectoryName(typeof(torch).Assembly.Location)!; | ||
| var torchGlobalDepsPath = Path.Combine(asmDir, "torch_global_deps.dll"); | ||
| var c10Path = Path.Combine(asmDir, "c10.dll"); | ||
| var uvPath = Path.Combine(asmDir, "uv.dll"); | ||
| var torchPath = Path.Combine(asmDir, "torch.dll"); | ||
| var torchCpuPath = Path.Combine(asmDir, "torch_cpu.dll"); | ||
| var libTorchSharpPath = Path.Combine(asmDir, "LibTorchSharp.dll"); | ||
| trace.AppendLine($" Attempting absolute-path load of native components from {asmDir}..."); | ||
| ok = TryLoadNativeLibraryFromFile(torchGlobalDepsPath, trace); | ||
| if (ok) ok = TryLoadNativeLibraryFromFile(c10Path, trace); | ||
| if (ok) ok = TryLoadNativeLibraryFromFile(uvPath, trace); | ||
| if (ok) ok = TryLoadNativeLibraryFromFile(torchPath, trace); | ||
| if (ok) ok = TryLoadNativeLibraryFromFile(torchCpuPath, trace); | ||
| if (ok) ok = TryLoadNativeLibraryFromFile(libTorchSharpPath, trace); | ||
| } |
There was a problem hiding this comment.
The absolute-path fallback loading for Windows CPU mode loads files even if earlier named-based loads succeeded (the check is if (!ok && isWindows)). However, this fallback will only execute if the previous named-based loads failed. The logic constructs paths for all dependency DLLs and attempts to load them in order. If any single load fails, the entire chain fails (the pattern if (ok) ok = ... means subsequent loads only happen if previous ones succeeded).
Consider whether this is the desired behavior, or if it should continue attempting to load remaining DLLs even if one fails, similar to how the named-based approach uses ok = ... without checking previous success for each individual library.
| <File Include="lib\aoti_custom_ops.dll"/> | ||
| <File Include="lib\backend_with_compiler.dll"/> | ||
| <File Include="lib\c10.dll"/> | ||
| <File Include="lib\jitbackend_test.dll"/> | ||
| <File Include="lib\torchbind_test.dll"/> | ||
| <File Include="lib\torch.dll"/> | ||
| <File Include="lib\torch_cpu.dll"/> | ||
| <File Include="lib\torch_global_deps.dll"/> | ||
| <File Include="lib\uv.dll"/> | ||
| <File Include="lib\armpl_lp64.dll"/> |
There was a problem hiding this comment.
The Windows ARM64 file paths use "lib" instead of "libtorch\lib" which is inconsistent with the x64 Windows paths (lines 34-41) and other platforms. This inconsistency suggests these files may be in a different directory structure in the ARM64 libtorch distribution. Please verify that the ARM64 libtorch archive actually has files directly in a "lib" folder rather than "libtorch\lib". If the directory structure is indeed different, this is correct. Otherwise, these paths should be updated to match the pattern used for other platforms.
| <File Include="lib\aoti_custom_ops.dll"/> | |
| <File Include="lib\backend_with_compiler.dll"/> | |
| <File Include="lib\c10.dll"/> | |
| <File Include="lib\jitbackend_test.dll"/> | |
| <File Include="lib\torchbind_test.dll"/> | |
| <File Include="lib\torch.dll"/> | |
| <File Include="lib\torch_cpu.dll"/> | |
| <File Include="lib\torch_global_deps.dll"/> | |
| <File Include="lib\uv.dll"/> | |
| <File Include="lib\armpl_lp64.dll"/> | |
| <File Include="libtorch\lib\aoti_custom_ops.dll"/> | |
| <File Include="libtorch\lib\backend_with_compiler.dll"/> | |
| <File Include="libtorch\lib\c10.dll"/> | |
| <File Include="libtorch\lib\jitbackend_test.dll"/> | |
| <File Include="libtorch\lib\torchbind_test.dll"/> | |
| <File Include="libtorch\lib\torch.dll"/> | |
| <File Include="libtorch\lib\torch_cpu.dll"/> | |
| <File Include="libtorch\lib\torch_global_deps.dll"/> | |
| <File Include="libtorch\lib\uv.dll"/> | |
| <File Include="libtorch\lib\armpl_lp64.dll"/> |
|
|
||
| static string nativeGlob => | ||
| RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? @".*\.dll" : | ||
| RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? @".*\.dll$" : |
There was a problem hiding this comment.
The regex pattern change adds a "$" end-of-string anchor to the Windows DLL pattern. However, at line 320 in the same file, this pattern is already wrapped with "^" and "$" anchors: var nativeRegExp = new Regex("^" + nativeGlob + "$");. This means the final regex will be ^.*\.dll$$ with a double dollar sign, which is incorrect and will fail to match DLL files. The "$" should be removed from the nativeGlob definition since it's already added when the regex is constructed.
| RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? @".*\.dll$" : | |
| RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? @".*\.dll" : |
| <LibTorchArchiveCoreName Condition="'$(TargetOS)' == 'windows'">libtorch-win-shared-with-deps$(LibTorchDebug)</LibTorchArchiveCoreName> | ||
| <LibTorchArchiveCoreName Condition="'$(TargetOS)' == 'linux'">libtorch-shared-with-deps</LibTorchArchiveCoreName> | ||
| <LibTorchArchiveCoreName Condition="'$(TargetOS)' == 'windows' and '$(TargetArchitecture)' == 'arm64'">libtorch-win-arm64-shared-with-deps$(LibTorchDebug)</LibTorchArchiveCoreName> | ||
| <LibTorchArchiveCoreName Condition="'$(TargetOS)' == 'linux'">libtorch-cxx11-abi-shared-with-deps</LibTorchArchiveCoreName> |
There was a problem hiding this comment.
The LibTorchArchiveCoreName for Linux has been changed to "libtorch-cxx11-abi-shared-with-deps", but there is no corresponding SHA file for the 2.10.0 version (libtorch-cxx11-abi-shared-with-deps-2.10.0%2Bcpu.zip.sha). The repository contains "libtorch-shared-with-deps-2.10.0%2Bcpu.zip.sha" but not the cxx11-abi variant for 2.10.0. This will cause the build to fail when trying to validate the downloaded archive. Either:
- The SHA file needs to be added to this PR, or
- This change to the Linux archive name should be removed if it was unintentional
This appears to be an unrelated change that was accidentally included in the Windows ARM64 support PR.
| <LibTorchArchiveCoreName Condition="'$(TargetOS)' == 'linux'">libtorch-cxx11-abi-shared-with-deps</LibTorchArchiveCoreName> | |
| <LibTorchArchiveCoreName Condition="'$(TargetOS)' == 'linux'">libtorch-shared-with-deps</LibTorchArchiveCoreName> |
| var isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); | ||
| if (isWindows) { | ||
| // Preload dependency chain explicitly on Windows: torch_global_deps -> c10 -> uv -> torch -> torch_cpu -> LibTorchSharp | ||
| trace.AppendLine($" Try loading Windows CPU native dependency chain: torch_global_deps -> c10 -> uv -> torch -> torch_cpu -> LibTorchSharp"); | ||
| ok = TryLoadNativeLibraryByName("torch_global_deps", typeof(torch).Assembly, trace); | ||
| if (ok) ok = TryLoadNativeLibraryByName("c10", typeof(torch).Assembly, trace); | ||
| if (ok) ok = TryLoadNativeLibraryByName("uv", typeof(torch).Assembly, trace); | ||
| if (ok) ok = TryLoadNativeLibraryByName("torch", typeof(torch).Assembly, trace); | ||
| if (ok) ok = TryLoadNativeLibraryByName("torch_cpu", typeof(torch).Assembly, trace); | ||
| if (ok) ok = TryLoadNativeLibraryByName("LibTorchSharp", typeof(torch).Assembly, trace); | ||
| } else { | ||
| ok = TryLoadNativeLibraryByName("torch_cpu", typeof(torch).Assembly, trace); | ||
| if (ok) ok = TryLoadNativeLibraryByName("LibTorchSharp", typeof(torch).Assembly, trace); | ||
| } |
There was a problem hiding this comment.
The new Windows CPU dependency loading chain is more explicit (torch_global_deps -> c10 -> uv -> torch -> torch_cpu -> LibTorchSharp) compared to the previous simpler approach that only loaded torch_cpu and LibTorchSharp. While this may be necessary for Windows ARM64, applying it to all Windows platforms (including x64) could be a breaking change. Consider:
- Whether this change is actually needed for Windows x64 or only for ARM64
- If this is needed for all Windows, testing on x64 to ensure backward compatibility
- Whether the order of dependencies is correct for all scenarios
If this change is specifically for ARM64 dependencies, consider adding a condition to only use this explicit chain for ARM64, or add a comment explaining why this is now needed for all Windows platforms.
Hi!
I add Windows ARM support with Copilot help (yea there can be some strange style and logic decigions) and check it on Snapdragon X Elite computer for my little project - it works.
Feel free point me or make changes directly.
Partially close #945
Thank you for this great project and make ML happen on dotnet easy!