-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Improvements for running on Windows with Snapdragon X #8531
Improvements for running on Windows with Snapdragon X #8531
Conversation
@AndreasKunar You can also find the overall procedure in the Github Actions Workflow
Once everything is installed build with
|
While I understand, some might use chocolately, winget is already shipping with Windows, and winget install cmake also does the job. I'd rather have both options available, than forcing chocoletely.
I'd rather also have the option to directly install llvm via github, without needing chocolately
I agree and should have included this! THANKS! Good input, I will update the doc tomorrow (Jul 18) morning my time (its late today, I'm tired/prone to errors, and I'd rather start fresh). |
No objection to having options :) So far, in the CI I used choco If winget is better option (ie included in windows by default, etc) I don't mind updating the CI to use winget. |
@max-krasnyansky - I have tried to clarify/simplify the instructions. The current VS2022 automatically installs cmake and ninja, so no need for separate installs (I also tested this on my PC). Clang can be either installed via download or choco. |
@max-krasnyansky apologies, I found an even easier installation method and changed the documentation again. Installing all via the VS2022 install (git, cmake, clang, MSVC and ninja) and documented it accordingly. I also suggest to disable openmp (-D GGML_OPENMP=OFF) with clang for arm64 and documented this. |
Looks good to me. Re: OpenMP. @fmz and I are working on the next round of the Threadpool updates that we submitted earlier. |
@ggerganov can you please point me to someone who can review+approve this? max-krasnyansky, the author of the initial Snapdragon X / Windows optimizations thinks its OK. Thanks in advance. |
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.
Do I understand correctly that:
- Building with
cmake --preset arm64-windows-llvm-release
the new aarch64 assembly kernels will compile and work correctly? - Building with
cmake --preset arm64-windows-MSVC
will also succeed, but the aarch64 assembly kernels will be skipped by the new conditionals?
building with MSVC breaks for Snapdragn X / Windows because MSVC does not support any C in-line Assembly for ARM (asm directive)
I also need someone to confirm this statement and that there is nothing better that we can do than disabling this code for certain environments (pinging @Dibakar)
Did you try using __asm
instead of __asm__
?
https://learn.microsoft.com/en-us/cpp/assembler/inline/asm?view=msvc-170
yes.
In MSVC all asm only works with 32-bit x86, and neither with x64 nor with arm64. @max-krasnyansky verified this in his comments. |
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
The conditional seems to be fine for now, I will confirm it shortly. |
@ggerganov - sorry for my inexperience with pull requests, and for my asking because I don't want to do anything wrong. Is there something I still need to do for this, or is everything automatic, once it got approved? Should I close the request? |
* Improvements for Windows with Snapdragon X * Revert "Improvements for Windows with Snapdragon X" This reverts commit bf21397. * Improvements for Windows with Snapdragon X * WOA build clarifications * WIndows on ARM build clarifications * cmake build for Windows clarifications * Update docs/build.md Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> --------- Co-authored-by: AndreasKunar <andreaskmsn.com> Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Question: Why is |
Its not disabled at all. But clang on Windoes says its both MSVC and clang. My conditional disables the asm only for "real" MSVC (MSVC and not clang) |
Improvements/Issues addressed:
ToDo in future fixes for Snapdragon X on Windows and not addressed by this PR:
Work on enabling GPU acceleration for SnapDragon X, e.g. via getting Vulkan to run on Snapdragon x - see Bug: llama.cpp with Vulkan not running on Snapdragon X + Windows (Copilot+PCs) #8455
Work on enabling NPU acceleration for SnapDragon X, e.g. via work on getting QNN to run, and also on Windows - see ggml-qnn: add Qualcomm QNN(Qualcomm Neural Network,aka Qualcomm AI Engine Direct) backend #6869 (currently only for Android)
P.S: This is my first PR ever, and with the help of chatGPT as tutor. Please have mercy on me, if I misunderstood things.