Stub of a new isel backend for x64

[bnjbvr]

This is a draft-ish PR containing a rebase of Julian's x64 work on the new isel backend, including quite a few refactorings of mine. During today's Cranelift meeting, we agreed that it might make sense to add the incomplete backend here, allowing incremental development by more contributors.

I've tried to split the files in a way that was similar to the aarch64 backend, but some things might still be wobbly; I might check this a bit further more tomorrow. Of course, that's also something we could do as follow-up PRs.

The new x64 backend is controlled by a new Cargo feature x64 (which also requires x86). Then, users of clif-util can use the --target x86_64 argument, and:

• get the code generated by the current isel (with legalization, Recipes, Encodings, etc.) and regalloc if nothing more is provided.
• or add --set use_new_backend and this will use the new isel backend. (Expect panics, unimplemented errors, etc.)

Second (small) commit is what allows passing ISA-specific flags to --set, which wasn't implemented yet.

[github-actions]

Subscribe to Label Action

cc @bnjbvr

This issue or pull request has been labeled: "cranelift", "cranelift:meta"

Thus the following users have been cc'd because of the following labels:

• bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[julian-seward1]

Overall this looks fine to me. My only real request is: I'd prefer that the instruction-fragment definitions (args.rs) not be split off from the main instruction definitions. I found this something of a nuisance for the arm64 version, having to flip back and forth between two source files when looking at the overall structure of a new instruction. Also, the win is less here -- x64 has fewer argument-fragment kinds than arm64.

[bnjbvr]

My only real request is: I'd prefer that the instruction-fragment definitions (args.rs) not be split off from the main instruction definitions.

I did quite like the separation of it, since the inst/mod.rs file then only contains the overall Inst enum and associated helpers (more needs to be moved from inst/mod to inst/args). Eventually it'll even out the sizes of these files to around 600 or 700 lines each, which is nice too. It also cleanly separates concepts (instructions vs operands), and blackboxes the operands (which are unlikely to change much over time, compared with the instructions for which we might add more variants more frequently). Navigating between files doesn't seem like a strong annoyance in this case, since only two are involved (and LSP makes it really easy to jump-to-definitions, I'd expect text editors have then a feature to jump back to the previous position). Overall, it seems better to keep the two files separated.

[abrown]

cc @jlb6740 @joshtriplett

[jlb6740]

• or add --set use_new_backend and this will use the new isel backend. (Expect panics, unimplemented errors, etc.)

Hi @bnjbvr. Do you think any example should show some signs of life here? I tried a simple module that adds to numbers with the current isel and got what I assume is expected results but running through the new isel there were no error messages or panics .. it just hung. Is this expected?

[bnjbvr]

Hi @bnjbvr. Do you think any example should show some signs of life here? I tried a simple module that adds to numbers with the current isel and got what I assume is expected results but running through the new isel there were no error messages or panics .. it just hung. Is this expected?

It's likely I went through compilation, but not actually running the code in wasmtime (since there seems to be no way to pass Cranelift CLI args through wasmtime runner).

I have used the following simple wasm module:

    (module
        (func (export "add") (param i32) (result i32)
            get_local 0
            i32.const 42
            i32.add
        )

compiled to wasm binary, and then, from Cranelift's directory (so as to use clif-util), ran the following:

cargo run -- wasm --target x86_64 --set use_new_backend=1 -dvTD /tmp/a.wasm # shows assembly generated with new backend
cargo run -- wasm --target x86_64 -dvTD /tmp/a.wasm # shows assembly generated with current backend

[bnjbvr]

Thanks all for the first round of comments! Many things we could do later on so as to improve this code (especially with respect to naming, and respecting Rust standards in general).

I just checked, and the simple module from the above comment works in Spidermonkey (with a few tweaks on the Spidermonkey side to make it use the new backend), which is a good sign.

[bjorn3]

Suggested change

	// VmContext is r14 in Baldrdash.
	// `VMContext` is `r14` in Baldrdash.

[bjorn3]

Suggested change

	pub(crate) fn get_enc(&self) -> u8 {
	*self as u8
	pub(crate) fn get_enc(self) -> u8 {
	self as u8

CC is copy

[jlb6740]

compiled to wasm binary, and then, from Cranelift's directory (so as to use clif-util), ran the following:

cargo run -- wasm --target x86_64 --set use_new_backend=1 -dvTD /tmp/a.wasm # shows assembly generated with new backend
cargo run -- wasm --target x86_64 -dvTD /tmp/a.wasm # shows assembly generated with current backend

Ahh thanks ... this worked. The difference was the ordering of the parameters.

This works:

cargo run -- wasm --target x86_64 --set use_new_backend=1 -dvTD /tmp/a.wasm

While this hung:

cargo run -- wasm --target x86_64 -dvTD --set use_new_backend=1 /tmp/a.wasm

[abrown]

As @bnjbvr said, there are plenty of things to improve but this seems like a good start. Once merged, it will be easier to iterate on this since the PRs should be fewer lines.

[whitequark]

What's the plan for supporting x86_32? The old backend was generic over the word size; this one doesn't seem to be. Does the new approach allow to make such a generic backend, or is it necessary to duplicate all the shared code?

[bnjbvr]

What's the plan for supporting x86_32? The old backend was generic over the word size; this one doesn't seem to be. Does the new approach allow to make such a generic backend, or is it necessary to duplicate all the shared code?

This question is still open for design. In my opinion, if it doesn't imply a huge cost on the x64 backend in terms of maintainability and readability, we should try to share and reuse as much as we can.

[bnjbvr]

Thanks for the reviews and new comments again! Will merge if CI passes.