-
Notifications
You must be signed in to change notification settings - Fork 280
transpile: tests: cross-transpile #1444
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
Open
kkysen
wants to merge
21
commits into
master
Choose a base branch
from
kkysen/cross-os-transpilation
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nerated-rust-toolchain.toml`
…ED_RUST_TOOLCHAIN_TOML` and use it to deduplicate
Contrary to its naming, `-fsyntax-only` checks semantics, too. It just doesn't emit anything, which is what we want.
`zig` isn't available through `apt`, but it is in PyPI, so we can install it with `uv`.
For cross-arch compilation, getting the right headers, sysroot, etc. is not, too, difficult, but for cross-os compilation, this is more difficult, as some OSes like macOS restrict these. `zig cc` contains headers for all possible C cross-compilation targets, so we use it to look them up.
For platform-specific snapshots, test in groups of all targets that match that platform (arch/os).
For Rust targets, they seem to match the clang target, at least for all of the targets we're testing now.
…ets, not necessarily all
…onger necessarily all
… them yet (32-bit has differences)
…builtin types are unhandled)
Some of these tests (`macros.c` and `rotate.c`) had to be further disambiguated in `ptr-width-specific` and `os-ptr-width-specific`. `vm_x86.c` also didn't compile after transpilation on `x86` due to an assembly error, so that's now commented out for now with a TODO.
…ve all of the `dummy.c`s The `dummy.c`s were needed because if an `insta::glob!` only found one path, then the common prefix was the whole thing and removed too much.
Also, print the `python` and `zig` versions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So I previously thought that Hayroll can handle cross-OS transpilation, just not cross-arch transpilation, but that turned out to be more of a fluke. Hayroll can only substantively handle cross-target conditional compilation if
c2rust transpilecan cross-transpile. Cross-transpiling is fairly easy for cross-arch transpilation since we just need to pass the right sysroot. But when trying to cross-OS transpile, getting that sysroot is much more difficult, namely for macOS.zig ccis an excellent C cross-compiler and it bundles headers for basically every possible target, so we can usezig ccto find out what args we need to pass tolibclangToolingto find the right headers to cross-transpile.c2rust transpileuseslibclangTooling, and it can't go throughzig ccdirectly, so we can callzig ccon an empty file with the-targetwe want and-###to print out the-cc1commands, which contains the-isystems,-Ds, etc. that we want. Then we can append those cross-compiling args toc2rust transpile's extra clang args. This lets us successfully cross-transpile on most targets.To start, I've added support for these Rust targets:
x86_64-unknown-linux-gnux86_64-apple-darwinaarch64-unknown-linux-gnuaarch64-apple-darwini686-unknown-linux-gnuarmv7-unknown-linux-gnueabihfriscv64gc-unknown-linux-gnuThese are the targets we cared about in
rav1d, for example.riscv64gc-unknown-linux-gnuisn't supported, though, since we don't handle a bunch of its builtin types yet, so while the tests are set up to handle it, we don't actually test on it.While testing on the others, I discovered some tests had to be further disambiguated, as they were different on the other targets we're now testing as well. The
UINTPTR_MAXfrommacros.chad to be moved toptr-width-specific/macros.csince it depends on the ptr width.os-specific/rotate.chad to be moved toos-ptr-width-specific/rotate.csince it was different depending on the ptr width in addition to the OS. Andvm_x86.c's transpiled code onx86didn't compile, so that part is commented out for now (I'll open an issue).For now, this is only done in
snapshots.rsfor the snapshot tests, as this was the easiest to start with. But we should be able to move a bunch of this intoc2rust transpileitself with a--targetoption so that cross-transpilation is very easy and Hayroll can use it to handle cross-target conditional compilation. And our snapshot tests should work much better now, sincecargo test -p c2rust-transpileruns all of them, and you don't have to wait for CI to tell you that a different platform is slightly different and update the snapshots manually.Also, since we're now cross-transpiling on 6 different targets,
cargo test -p c2rust-transpiletakes significantly longer (about 6x longer). Butc2rust_transpile::transpileis not parallelizable within the same process, so we can't easily parallelize this. This was an old issue I remember running into, and it's due to some weird waylibclangToolingworks and stores stuff internally.