Add support for CLANGARM64 target #3916

dennisameling · 2022-06-24T18:25:21Z

This requires an ARM64-machine with Windows 11 installed (which supports x64 emulation for MSYS2)

This is basically a successor of #3427 but stays very close to Git for Windows's SDK and integration with MSYS2. Thanks to the work the MSYS2 team did on clangarm64, this actually becomes very easy to do now!

The main idea

Use the main MSYS2/git-sdk-64 setup, which works on Windows 11 on ARM thanks to x64-emulation
Configure the official clangarm64 MSYS2 repo
Install mingw-w64-clang-aarch64-toolchain which contains the ARM64-native Clang compiler

CI results

Successful CI run on my ARM64 machine: https://github.com/dennisameling/git/actions/runs/2557291542. The make -j8 command took 3m9s on my machine, which I think is very decent.

Just to be 100% clear that the compiler itself is indeed ARM64-native:

Note that gcc.exe is basically a link to clang.exe on MSYS2 clangarm64.

Remarks/questions

Shall I leave out the CI pipeline from this PR (or make it a manual job through workflow_dispatch so folks can run it from their forks), as it requires a native ARM64 device? Note that Azure now has ARM64 VMs in public preview - including Windows 11 Pro/Enterprise - so self-hosted GitHub Runners on Azure ARM64 VMs are theoretically possible already. I simply installed a self-hosted runner on my Surface Pro X in a VM.
Would it make sense to create a git-sdk-ARM64 with x64 MSYS2 and the clangarm64 packages shown in the attached workflow? I updated setup-git-for-windows-sdk with some basic support for this scenario (as a POC).
I disabled USE_NED_ALLOCATOR on CLANGARM64 as I ran into errors. The resulting binary works just fine in my limited testing - would you consider a build without nedmalloc a feasible option for a "first version with ARM64 support"? Happy to do some performance testing if you could point me to some commands I can test.
I'll need to do quite some updates to build-extra in order to add support for building the release packages (mingit, installer, etc.). As mentioned in this issue before, MSVC builds are problematic due to the VC++ redistributable requirement. Should we consider that option "off the table" and work towards this MSYS2/clangarm64 solution instead? Just want to make sure that I'm implementing a clean solution in build-extra and leaving the MSVC-scenario out of that would probably make things a bit simpler.

$ /clangarm64/bin/git --version --build-options
git version 2.37.0.rc2.windows.1.7.g45a475aeb8.dirty
cpu: aarch64
built from commit: 45a475aeb84a53a6432b1ede716a4b6f05e11947
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon

dennisameling · 2022-06-24T18:32:15Z

.github/workflows/clang-build.yml

+      NO_PERL: 1
+    steps:
+      - uses: actions/checkout@v3
+      - uses: dennisameling/setup-git-for-windows-sdk@01872063f7b133eae0732176f08e309a69c22b4d


git-for-windows/setup-git-for-windows-sdk#397

dscho · 2022-06-25T21:44:24Z

Great work!

Shall I leave out the CI pipeline from this PR (or make it a manual job through workflow_dispatch so folks can run it from their forks), as it requires a native ARM64 device? Note that Azure now has ARM64 VMs in public preview - including Windows 11 Pro/Enterprise - so self-hosted GitHub Runners on Azure ARM64 VMs are theoretically possible already. I simply installed a self-hosted runner on my Surface Pro X in a VM.

It would be best to leave it out by default, for now, otherwise we will end up with PRs in this state:

Would it make sense to create a git-sdk-ARM64 with x64 MSYS2 and the clangarm64 packages shown in the attached workflow? I updated setup-git-for-windows-sdk with some basic support for this scenario (as a POC).

Maybe in the long run, I would think. Or is it that expensive to run the pacman command to download and install clang?

I disabled USE_NED_ALLOCATOR on CLANGARM64 as I ran into errors. The resulting binary works just fine in my limited testing - would you consider a build without nedmalloc a feasible option for a "first version with ARM64 support"? Happy to do some performance testing if you could point me to some commands I can test.

My answer to that is still the same as before. My preference would be to integrate mimalloc, but I simply lack the time to take on that task.

As mentioned in this issue before, MSVC builds are problematic due to the VC++ redistributable requirement. Should we consider that option "off the table" and work towards this MSYS2/clangarm64 solution instead?

The MSYS2/clangarm64 solution indeed sounds like the most promising way forward.

Again, thank you for keeping up the good work!

dscho · 2022-09-08T18:19:11Z

My preference would be to integrate mimalloc, but I simply lack the time to take on that task.

True to my words, it took a long good while until I got it done, but done I got it.

dennisameling · 2022-09-08T20:56:38Z

True to my words, it took a long good while until I got it done, but #4013.

That's fantastic news. Thanks for picking it up!

I had quite a lot of things on my plate over the last few weeks, but let me try to pick up the work on this next week, now that mimalloc is in place. Very much motivated to push this over the finish line. ARM64 Azure VMs have reached General Availability on September 1st, and there's been some more exciting developments in the ecosystem. Will keep you posted!

dscho · 2022-09-12T14:39:01Z

ARM64 Azure VMs have reached General Availability on September 1st, and there's been some more exciting developments in the ecosystem.

Cool! Thank you for notifying me, I had missed that announcement.

Will keep you posted!

Sounds good!

dennisameling

@dscho This should be ready for review now 😊

dennisameling · 2022-10-04T08:37:13Z

compat/bswap.h

+/* available since Clang 12 and GCC 10, i.a. needed for ARM64/aarch64 on Windows */
+#elif defined(__has_builtin) && __has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64)
+
+#define bswap32(x) __builtin_bswap32((x))
+#define bswap64(x) __builtin_bswap64((x))


I wonder whether we want to make this the first item in the if block (before #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))). Don't we always want to prefer built-ins if they're available? The original code in this file was written 13 years ago, long before those built-ins were available. What do you think?

I agree, this would make sense. This patch could then even be contributed to the Git project individually, as it can stand on its own.

Ok, let's update it then. Will update the commit in a few hours from now. Thanks for the review!

@dscho Updated. I had to change the logic a bit because the __has_builtin macro is not defined on older compiler versions, and was throwing errors in CI because it still uses GCC 8. What do you think of the updated approach?

dennisameling · 2022-10-04T08:38:05Z

.github/workflows/clangarm64-build.yml

+name: CLANG build ARM64
+
+on:
+  workflow_dispatch:
+
+defaults:
+  run:
+    shell: bash
+
+jobs:
+  clang-build:
+    runs-on: [Windows, ARM64]
+    env:
+      NO_PERL: 1
+    steps:
+      - uses: actions/checkout@v3
+      - uses: git-for-windows/setup-git-for-windows-sdk@v1
+        with:
+          flavor: makepkg-git
+          architecture: aarch64
+          # This assumes that the job is running on a self-hosted runner,
+          # in which case we need to cleanup SDK files.
+          cleanup: true
+      - name: Build Git CLANGARM64
+        run: make -j`nproc`


Successful run: https://github.com/dennisameling/git/actions/runs/3180550159/jobs/5184383062

dscho

@dennisameling this is exciting!

I'll leave it up to you whether or not you want to rework the bswap.h patch before merging. Just let me know where we want to go from here.

dscho · 2022-10-06T15:54:50Z

compat/bswap.h

+/* available since Clang 12 and GCC 10, i.a. needed for ARM64/aarch64 on Windows */
+#elif defined(__has_builtin) && __has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64)
+
+#define bswap32(x) __builtin_bswap32((x))
+#define bswap64(x) __builtin_bswap64((x))


I agree, this would make sense. This patch could then even be contributed to the Git project individually, as it can stand on its own.

Newer compiler versions, like GCC 10 and Clang 12, have built-in functions for bswap32 and bswap64. This comes in handy, for example, when targeting CLANGARM64 on Windows, which would not be supported without this logic. Signed-off-by: Dennis Ameling <[email protected]>

CLANGARM64 is a relatively new MSYSTEM added by the MSYS2 team. In order to have Git build correctly for this platform, let's add some configuration for it to config.mak.uname. Signed-off-by: Dennis Ameling <[email protected]>

No GitHub-hosted ARM64 runners are available at the moment of writing, but folks can leverage self-hosted runners of this architecture. This CI pipeline comes in handy for forks of the git-for-windows/git project that have such runners available. The pipeline can be kicked off manually through a workflow_dispatch. Signed-off-by: Dennis Ameling <[email protected]>

dscho · 2022-10-28T15:49:28Z

@dennisameling thank you so much for keeping up the good work despite all the obstacles (the delays caused by my unavailability being one).

**This requires an ARM64-machine with Windows 11 installed (which supports x64 emulation for MSYS2)** ### The main idea - Use the main MSYS2/git-sdk-64 setup, which works on Windows 11 on ARM thanks to x64-emulation - Configure the official `clangarm64` MSYS2 repo - Install `mingw-w64-clang-aarch64-toolchain` which contains the ARM64-native Clang compiler

dennisameling force-pushed the windows-arm64-clangarm64 branch from edba948 to db4cf0a Compare June 24, 2022 18:27

dennisameling mentioned this pull request Jun 24, 2022

Add ARM64/aarch64 support git-for-windows/setup-git-for-windows-sdk#397

Merged

dennisameling commented Jun 24, 2022

View reviewed changes

dennisameling force-pushed the windows-arm64-clangarm64 branch from db4cf0a to 9eae59e Compare September 17, 2022 09:54

dennisameling force-pushed the windows-arm64-clangarm64 branch 2 times, most recently from af47dc5 to 51073fd Compare October 4, 2022 08:35

dennisameling commented Oct 4, 2022

View reviewed changes

dscho approved these changes Oct 6, 2022

View reviewed changes

dennisameling force-pushed the windows-arm64-clangarm64 branch 3 times, most recently from 16ed971 to 9e9345c Compare October 7, 2022 07:14

dennisameling added 3 commits October 28, 2022 15:34

config.mak.uname: add support for clangarm64

b6421d3

CLANGARM64 is a relatively new MSYSTEM added by the MSYS2 team. In order to have Git build correctly for this platform, let's add some configuration for it to config.mak.uname. Signed-off-by: Dennis Ameling <[email protected]>

dscho force-pushed the windows-arm64-clangarm64 branch from 9e9345c to c1cc10c Compare October 28, 2022 13:35

dscho merged commit bb3e903 into git-for-windows:main Oct 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for CLANGARM64 target #3916

Add support for CLANGARM64 target #3916

dennisameling commented Jun 24, 2022 •

edited

Loading

dennisameling Jun 24, 2022

dscho commented Jun 25, 2022

dscho commented Sep 8, 2022

dennisameling commented Sep 8, 2022

dscho commented Sep 12, 2022

dennisameling left a comment

dennisameling Oct 4, 2022

dscho Oct 6, 2022

dennisameling Oct 6, 2022

dennisameling Oct 7, 2022

dennisameling Oct 4, 2022

dscho left a comment

dscho Oct 6, 2022

dscho commented Oct 28, 2022

Add support for CLANGARM64 target #3916

Add support for CLANGARM64 target #3916

Conversation

dennisameling commented Jun 24, 2022 • edited Loading

The main idea

CI results

Remarks/questions

dennisameling Jun 24, 2022

Choose a reason for hiding this comment

dscho commented Jun 25, 2022

dscho commented Sep 8, 2022

dennisameling commented Sep 8, 2022

dscho commented Sep 12, 2022

dennisameling left a comment

Choose a reason for hiding this comment

dennisameling Oct 4, 2022

Choose a reason for hiding this comment

dscho Oct 6, 2022

Choose a reason for hiding this comment

dennisameling Oct 6, 2022

Choose a reason for hiding this comment

dennisameling Oct 7, 2022

Choose a reason for hiding this comment

dennisameling Oct 4, 2022

Choose a reason for hiding this comment

dscho left a comment

Choose a reason for hiding this comment

dscho Oct 6, 2022

Choose a reason for hiding this comment

dscho commented Oct 28, 2022

dennisameling commented Jun 24, 2022 •

edited

Loading