-
Notifications
You must be signed in to change notification settings - Fork 45
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
shrink LLVM #75
shrink LLVM #75
Conversation
Oh, I messed up and didn't add all the changes. That explains that. |
c36a977
to
3e45a92
Compare
What is going on with this patch? I get an sha256 of |
We want to build our own smaller Clang for ARM64, teach open-pr how to update it. This update script uses the update-clang-from-msys2.sh script from git-for-windows/MINGW-packages#75 Signed-off-by: Matthias Aßhauer <[email protected]>
after merging git-for-windows/MINGW-packages#75 we'll want to be notified when Msys2 updates their mingw-w64-clang package. Signed-off-by: Matthias Aßhauer <[email protected]>
Good news The Good news: I have access to an ARM64 machine to build and test on now.
The 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.
Great work!
I'm somewhat curious about that changing SHA-256. My guess is that the file sometimes contains CR/LF and sometimes not?
I left a couple of suggestions for the update script. Please let me know if you find them useful.
test -n "$old_pkgrel" || | ||
die "$0: could not determine current pkgrel\n" | ||
|
||
git clone --sparse --depth 1 https://github.com/msys2/MINGW-packages upstream |
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.
How about --filter=blob:none
? That makes the clone really fast, at the expense of adding a second (implicit) fetch in the next git
invocation.
rm -f *.patch | ||
mv upstream/$pkgname/*.patch ./ | ||
rm -f PKGBUILD | ||
mv upstream/$pkgname/PKGBUILD ./ | ||
rm -f README-patches.md | ||
mv upstream/$pkgname/README-patches.md ./ |
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.
These invocations probably need some &&
-chained error checking.
The toolchain from this does successfully build |
What does the output of |
This comment was marked as resolved.
This comment was marked as resolved.
Seems that our construction of a long path is incorrect... I guess that the three letters |
They do make the difference |
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.
Thank you so much for all your work on this! 🚀 I won't have my arm64 device at hand until Tuesday or Wednesday, but happy to test later if that helps.
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.
I just built this using makepkg-mingw
. The total build took ~2 hours on my 8-core Surface Pro X (ARM64). Note that ~1,5 hours was the actual time it took to build using clang
, which used 100% of the CPU. All the rest is a bunch of processing in bash
and other tools that are x64 emulated and thus are rather slow on ARM64.
The ARM64 libLLVM-16.dll comes out at 47641k.
I can confirm that I'm also seeing 47641k on my machine after the build. Awesome!
Thank you so much for your work on this! I guess we somehow need to make sure that git-for-windows-automation
will keep this up to date. Is there any package list or reference which lists the MINGW-packages
that Git for Windows is supposed to build itself? Or is it just a matter of kicking off a build-and-deploy
pipeline after this PR has been merged, which would then upload the packages to GfW's own Pacman repos?
The closest thing to such a list is the git-for-windows/git#4487 adds clang to that "list".
Exactly. |
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.
@rimrul very good, let's roll with this!
Looks like |
Ah, that makes sense! |
Updated to latest Msys2 range-diff:
|
And they just updated to 17.0.2 and plan to add a backport for arm64 soon that will bump |
update clang to 17.0.2-2 from Msys2 in preparation for building our own smaller clang for ARM64. Signed-off-by: Matthias Aßhauer <[email protected]>
So what do you think is the best course of action here, merge now, or merge after they bumped |
msys2#18690 has been merged, so that bump happened. This is updated to that version, so we're almost ready to merge. I think somewhere along the way I lost the safety pkgrel bump on our side, though. I originally had a pkgrel bump in 3c2fd59 to ensure our version was always newer than upstream, but that's not in a45aa33 anymore. |
Msys2 builds pretty versatile Clang and LLVM packages with LLVM backends to cross compile for various target architectures. That comes at the cost of big binaries, that challenge how we manage the Git for Windows SDK[1]. But for Git for Windows we don't need all of this. We only need a Clang that can compile ARM64 binaries on ARM64. [1] git-for-windows/git-sdk-arm64#7 Signed-off-by: Matthias Aßhauer <[email protected]>
We'll need to update our clang when Msys2 updates theirs and reapply our customisations every time, so a script makes this job less tedious and leaves no room to miss a step of the process. This script performs the same steps as the previous two commits. (it was used to generate the files for those) Signed-off-by: Matthias Aßhauer <[email protected]>
I've added the bump back in, so we should be ready to merge. |
Great! Please do deploy and merge at your leisure @rimrul! |
/deploy mingw-w64-clang The workflow run was started. |
From git-for-windows/git-sdk-arm64@63f00c0: Very nice! |
And there are no more >100MB files: https://github.com/git-for-windows/git-sdk-arm64/actions/runs/6569383663/job/17845204550#step:5:36 (compare to the previous |
We want to build our own smaller Clang for ARM64, teach open-pr how to update it. This update script uses the update-clang-from-msys2.sh script from git-for-windows/MINGW-packages#75 Signed-off-by: Matthias Aßhauer <[email protected]>
We want to build our own smaller Clang for ARM64, teach open-pr how to update it. This update script uses the update-clang-from-msys2.sh script from git-for-windows/MINGW-packages#75 Signed-off-by: Matthias Aßhauer <[email protected]>
The current ARM64 LLVM from Msys2 comes with files over 100 MB, GitHubs filesize limit without using LFS. We've slightly reduced the size using
strip
, but it's only a matter of time before we'll run into the same issue again.We can build our own smaller Clang/LLVM to better suit how we manage the Git for Windows SDK.