Deduplicate test suites for alternative codegen backends

[RalfJung]

Currently, the cranelift and gcc codegen backends have tiny test suites in their respective folders that get run in rustc CI:

• https://github.com/rust-lang/rust/tree/ec0b3540925af3cab9c38b0344902d48edb9d5b8/compiler/rustc_codegen_cranelift/example
• https://github.com/rust-lang/rust/tree/ec0b3540925af3cab9c38b0344902d48edb9d5b8/compiler/rustc_codegen_gcc/example

My understanding is that these are basically smoke-tests, but they use a lot of low-level internal and unstable APIs that keep changing, making them a maintenance hazard. As you can easily see, there's also a lot of duplication between them. I don't even know often I had to fix the mini_core tests (looks like both backends have two of them, for some reason). PRs like #128731 would benefit from adding another test there (ui/simd/shuffle.rs, in that case), but I don't want to add even more duplication.

Is there any way we can deduplicate this? The first idea that comes to my mind is that instead of having these example files stored as copies in the respective backends, we have a run-tests.txt file (or so) that lists a bunch of filenames relative to tests/ui that should be built and run with the backend. That means

• One can easily ensure that the test itself type-checks and works as intended using ./x.py test ui, without even having to do the setup required for the alternative codegen backend.
• The test is shared between all backends, reducing duplication.
• For cases like simd_shuffle intrinsic: allow argument to be passed as vector #128731, one can just easily add another test to the list, when it turns out there is an interesting special case to cover.

Cc @bjorn3 @antoyo @GuillaumeGomez

[antoyo]

I'm all in favor of removing the duplication here.

For tests like ui/simd/shuffle.rs, as mentionned in the linked PR, we do not currently run the UI tests for cg_gcc (I do not know if they are ran for cranelift) in the Rust repo (we only run them in the cg_gcc repo for now) and while we would want to have them ran as soon as possible in the CI of the Rust repo, the process to getting there is very slow (currently waiting for this PR to be merged) and any help (to review and answer the questions of people about their concerns — for instance, licencing) would be very appreciated.

Since cg_gcc gets frequently broken by changes in the Rust repo (because most cg_gcc's tests are not ran in its CI), I do like to have the ability to run tests that do not require compiling the sysroot since it's often broken, so having tests without it make it easier to debug. I do not know if your idea of run-tests.txt would change this.

[RalfJung]

Since cg_gcc gets frequently broken by changes in the Rust repo (because most cg_gcc's tests are not ran in its CI), I do like to have the ability to run tests that do not require compiling the sysroot since it's often broken, so having tests without it make it easier to debug. I do not know if your idea of run-tests.txt would change this.

Having a single mini_core file in the repo (and having it in the ui test suite) instead of 4 would already be a good improvement. ;)

[RalfJung]

For tests like ui/simd/shuffle.rs, as mentionned in the linked PR, we do not currently run the UI tests for cg_gcc (I do not know if they are ran for cranelift) in the Rust repo (we only run them in the cg_gcc repo for now) and while we would want to have them ran as soon as possible in the CI of the Rust repo

Clearly there's some logic somewhere that iterates the files in examples/ and builds them with the GCC backend and runs them. It seems like a very small change to instead read run-tests.txt and iterate the file listed there, and run them the same way. That doesn't seem to require any big infrastructure changes?

[antoyo]

Clearly there's some logic somewhere that iterates the files in examples/ and builds them with the GCC backend and runs them. It seems like a very small change to instead read run-tests.txt and iterate the file listed there, and run them the same way. That doesn't seem to require any big infrastructure changes?

Doing that would be OK, unless you're doing so in Rust and somehow use a sysroot compiled with cg_gcc to compile this Rust program iterating over the tests.

Having a single mini_core file in the repo (and having it in the ui test suite) instead of 4 would already be a good improvement. ;)

Not if this requires to compile the sysroot with cg_gcc, which I believe is the case for UI tests. There might be a possibility of using a LLVM-compiled sysroot in this case, though.

All I'm saying is that we should keep the ability to run these tests without having to compile the sysroot with cg_gcc. Do you know if this would be possible?

[RalfJung]

(replying here as that seems like a better place)

having the standard output of rustc test suites require the sysroot and those tests are there to help when compiling the sysroot with cg_gcc gets broken by a change, so that's why they're not standard tests. Perhaps we could workaround this by compiling the sysroot with cg_llvm in this case.

No this doesn't require a GCC-built sysroot I think. You need to use the libtest harness or something similar but you don't need to build it with GCC. The test harness should be an LLVM-built binary which, for each test file, invokes rustc with the GCC backend.

Probably the easiest way to do this is using the ui_test crate, which Miri and Clippy also use.

[bjorn3]

The tests are linked against the standard library, which needs either a cg_llvm compiled standard library which is abi compatible with your codegen backend, or needs to build the standard library using your codegen backend. Neither of which is the case during early development of a codegen backend.

[antoyo]

The tests are linked against the standard library, which needs either a cg_llvm compiled standard library which is abi compatible with your codegen backend, or needs to build the standard library using your codegen backend. Neither of which is the case during early development of a codegen backend.

I was talking about some of these tests and the mini-core tests.

[RalfJung]

Doing that would be OK, unless you're doing so in Rust and somehow use a sysroot compiled with cg_gcc to compile this Rust program iterating over the tests.

There is currently a program doing the iteration, whatever language it is written in surely can also do that based on a file. Also, there are literally tests there that depend on std? E.g. "std_example". Something is clearly setting up a suitable sysroot already?

[antoyo]

Also, there are literally tests there that depend on std? E.g. "std_example". Something is clearly setting up a suitable sysroot already?

Perhaps I was not clear: while some tests depend on the std, some don't and I'd like to keep a way to run those tests without requiring compiling the sysroot with cg_gcc to help debugging.

[RalfJung]

Yeah seems totally fine to keep a single mini_core file that can be included by tests that to not want to depend on a sysroot (ideally with a comment in that file explaining why). I don't see how that is in conflict with moving the tests to a place where they can be shared across backends. We already have other #![no_std] tests in tests/ui, maybe mini_core can then even be shared with them (via include!/#[path]).

But it shouldn't be necessary to have 4 copies of this that all need to be updated when something abut these very unstable lang items changes.

[bjorn3]

The reason we need multiple copies of mini_core is because it needs to be available in the rust-lang/rustc_codegen_cranelift and rust-lang/rustc_codegen_gcc repos for their respective CI to be able to use it. If it were to be put in some common location in the rust repo, it wouldn't be available in the rust-lang/rustc_codegen_cranelift and rust-lang/rustc_codegen_gcc repos. Where are you counting 4 copies of mini_core by the way? Both cg_clif and cg_gcc should only have a single copy in example/mini_core.rs.

[RalfJung]

The reason we need multiple copies of mini_core is because it needs to be available in the rust-lang/rustc_codegen_cranelift and rust-lang/rustc_codegen_gcc repos for their respective CI to be able to use it.

It's not great to ask rustc contributors to keep all these copies in sync, though. There's a tradeoff here between making life easier for rustc contributors vs making life easier for people working in the backend repo. I am obviously biased here since I don't work in the backend repos at all, but the current situation is tedious for rustc contributors. On top of the fact that duplication is inherently undesirable and increases maintenance cost, there are test files strewed about in random subfolders that nobody expects (examples doesn't even sound like a test folder), the failures will never be noticed until PR CI time, and then I generally do random changes locally until PR CI is green since I can never remember how to run these tests locally (and at least for GCC this requires quite non-trivial setup AFAIK) -- this often adds several CI roundtrips to a PR, not a pleasant experience at all. In no case I experienced was the failure even related to the backend at all, it was always some change I did to a lang item or intrinsic that requires the code to be changed so that it still builds. That's why it would help to have these files included in the regular runs with the LLVM backend.

So IMO we should find a better solution here, and IMO slightly increasing the burden on backend maintainers is an okay tradeoff if it makes life better for (a much larger number of) rustc contributors.
I am not sure what the best solution here looks like. Maybe the test script in the rustc_codegen_cranelift repo could do a (shallow) clone of the rustc repo to fetch the files from there? Maybe whatever sync script you are using to sync changes from this repo back to your repos copies over the relevant files from tests/ui? This feels like it should have a reasonable technical solution.

Where are you counting 4 copies of mini_core by the way?

There's two mini_core.rs and two mini_core_hello_world.rs... but now that I look at it, the latter seem to import the former somehow, so I guess it's "just" two (plus similar file(s) in the ui test suite).

[RalfJung]

There's another minicore at tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs. So it's 3 copies total in the repo.

[antoyo]

I'm not sure what solution would work, to be honest.
Perhaps @bjorn3 has an idea?