-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add support for building for Windows on ARM64, natively. #13116
base: master
Are you sure you want to change the base?
Conversation
Cherry-picked from prusa3d/PrusaSlicer@b11b454 Ref: prusa3d/PrusaSlicer#13116
Cherry-picked from prusa3d/PrusaSlicer@b11b454 Ref: prusa3d/PrusaSlicer#13116
Cherry-picked from prusa3d/PrusaSlicer@b11b454 Ref: prusa3d/PrusaSlicer#13116 Co-authored-by: Hernan Martinez <[email protected]>
Can't you use ARM64X to fix the cross compatibility issue? |
It's already working, everything fully native. |
I believe they meant "to enable cross-compilation". |
Well that's not what ARM64EC/ARM64X are used for. Those are ABI types. It has nothing to do with the ability to build an ARM64, ARM64EC or ARM64X binary from a x64 host machine. (The latter is which I mean by cross-compilation). |
@hmartinez82 Excellent work! What is the minimal command line invocation I need to try building this on my machine? |
@JustinBeckerMSFT from a native ARM64 Developer Command Prompt in a ARM64 machine, the build instructions are the same as the manual x64 ones as detailed here: https://github.com/prusa3d/PrusaSlicer/blob/master/doc/How%20to%20build%20-%20Windows.md#2a-manual-build-instructions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @hmartinez82,
thank you very much for your work. It is a very valuable addition which we intend to use to enable building for windows on ARM64. It seems everything you proposed look good!
From our position we have the luxury to update some dependencies (something we want to do anyway) so some of the patches will not be necessary. I added notes about the versions in the review.
Also the state of cmake around mpfr + gmp on windows can be described as "not great". We intend to take arm64 into consideration when addressing it.
The final thing is that we will have to create/source our own gmp and mpfr dlls as we cannot just include any binary in our codebase for security reasons. I am absolutely certain that your intentions are pure, so please do not take it personally.
-#if defined(BOOST_JSON_HAS_MSVC_32BIT_INTRINSICS) | ||
+#if defined(BOOST_JSON_HAS_MSVC_32BIT_INTRINSICS) && !defined(_M_ARM64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in boost 1.84.0 so we will rather update boost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SachCZ I'm finally back. When do you think this will happen. I can rebase this branch (I'm 280+ commits behind). And I would like to keep the branch in a buildable state.
target_compile_definitions (glew PRIVATE "GLEW_BUILD;VC_EXTRALEAN") | ||
target_compile_definitions (glew_s PRIVATE "GLEW_STATIC;VC_EXTRALEAN") | ||
- target_link_libraries (glew LINK_PRIVATE -BASE:0x62AA0000) | ||
+ if(CMAKE_SYSTEM_PROCESSOR MATCHES "^(i?86|x86|x86_32)$") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future reference: fix is in glew master but it is not released yet.
set(_output ${_dstdir}/include/gmp.h | ||
${_dstdir}/lib/libgmp-10.lib | ||
${_dstdir}/bin/libgmp-10.dll) | ||
set(_output ${_dstdir}/include/gmp.h) | ||
|
||
if (CMAKE_SYSTEM_PROCESSOR MATCHES "^(ARM64|aarch64)$") | ||
list(APPEND _output ${_dstdir}/lib/gmp.lib | ||
${_dstdir}/bin/gmp-10.dll) | ||
set(_gmplib winarm64/gmp.lib) | ||
set(_gmpdll winarm64/gmp-10.dll) | ||
else() | ||
list(APPEND _output ${_dstdir}/lib/libgmp-10.lib | ||
${_dstdir}/bin/libgmp-10.dll) | ||
set(_gmplib win${DEPS_BITS}/libgmp-10.lib) | ||
set(_gmpdll win${DEPS_BITS}/libgmp-10.dll) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the near future we want to refactor the cmake so it uses an interface target to link against the pre-compiled dlls. We will move these ifs there.
-#if defined __SSE2__ || (_MSC_VER >= 1300 && !_M_CEE_PURE) | ||
+#if defined __SSE2__ || (_MSC_VER >= 1300 && !_M_CEE_PURE) && !defined(_M_ARM64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is hopefully fixed in OpenEXR 3.2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently so, the file is now way more comprehensive and even has NEON optimizations https://github.com/AcademySoftwareFoundation/openexr/blob/v3.3.0/src/lib/OpenEXR/ImfSimd.h
Understood. I'll remove the binaries and rebase this branch on top of master. |
b11b454
to
6cfe2dc
Compare
6cfe2dc
to
fda7436
Compare
This adds the required changes to build for Windows on ARM on a Windows on ARM machine. No cross compilation supported yet.
Notable changes:
Fixes #13107 and #12291