Remove cross compilation and enable more architectures #32
Conversation
dabrahams
left a comment
dabrahams
left a comment
There was a problem hiding this comment.
I don't remember what cross-compilation was in there for. Please check
- with @compnerd, from whose work this is derived, about his reasons for using cross-compilation in the first place
- that this change doesn't slow the process down too much. That's not a very high bar if we're running this code infrequently anyway.
A couple years ago GitHub didn't offer such a wide range of runners. I guess this was at least one reason. |
|
Yeah, the ARM64 runners were not available until recently. The cross-compilation allowed us to generate the binaries even if we could not test/run them. |
| run: | | ||
| pwsh -File ${{ github.workspace }}/SourceCache/llvm-build/scripts/fix-vs.ps1 | ||
| - name: Test binary compatibility |
There was a problem hiding this comment.
What does this check (aside from its diagnostic output)? That the llvm-config we built runs on the runner that built it?
There was a problem hiding this comment.
Yes, previously this kind of problem was only detected by a later step that made the .pc file by calling llvm-config. These are the runners that we are using for the hylo-new builds, so it's good to have a postcondition check that the tool is not broken. Making the .pc file also invokes llvm-config, but that's just incidental, this postcondition check is explicitly there to check if binaries are not corrupted or accidentally compiled in a way that makes them fail to run.
| echo "=== Testing Binary Compatibility ===" | ||
| echo "Runner OS: ${{ runner.os }}" | ||
| echo "Runner Arch: ${{ runner.arch }}" | ||
| echo "Target Arch: ${{ matrix.arch }}" | ||
| echo "Target Triple: ${{ matrix.triple_cpu }}-${{ matrix.triple_suffix }}" | ||
| echo "" | ||
| # Test llvm-config | ||
| echo "Testing llvm-config..." | ||
| "${{ github.workspace }}/BuildRoot/${{ env.PACKAGE_NAME }}/bin/llvm-config${{ matrix.executable_suffix }}" --version | ||
There was a problem hiding this comment.
Some or all of the diagnostic printing looks like AI-generated slop. Under what circumstances would it be useful?
| - name: Setup sccache | ||
| id: sccache | ||
| uses: compnerd/ccache-action@sccache-0.7.4 | ||
| uses: hendrikmuhs/ccache-action@v1.2.20 |
There was a problem hiding this comment.
Not opposed to changing ccache vendors but I'd like to understand why we're doing it.
There was a problem hiding this comment.
Compnerd's fork was no longer necessary, the build succeeded and the speedup was at least noticeably with sccache than without. This is the upstream repository which is actively maintained.
I am not sure about the reliability of sccache though, or in the timing of the the builds being consistent. Some targets were significantly faster with a cache hit than without, while some others got speed up. I'll rerun the build a couple more times to see if there is a consistent pattern.
Co-authored-by: Dave Abrahams <[email protected]>
Co-authored-by: Dave Abrahams <[email protected]>
Solves issue #31 .
Related PRs for testing the resulting library in other repos:
See the action run from creating the distributable packages: https://github.com/hylo-lang/llvm-build/actions/runs/20003814931/job/57362991952
The resulting release: https://github.com/hylo-lang/llvm-build/releases/tag/20251207-115516
This should be squashed when merging.