Skip to content

Commit

Permalink
cranelift: Add newtype wrappers for x64 register classes
Browse files Browse the repository at this point in the history
This primary motivation of this large commit (apologies for its size!) is to
introduce `Gpr` and `Xmm` newtypes over `Reg`. This should help catch
difficult-to-diagnose register class mixup bugs in x64 lowerings.

But having a newtype for `Gpr` and `Xmm` themselves isn't enough to catch all of
our operand-with-wrong-register-class bugs, because about 50% of operands on x64
aren't just a register, but a register or memory address or even an
immediate! So we have `{Gpr,Xmm}Mem[Imm]` newtypes as well.

Unfortunately, `GprMem` et al can't be `enum`s and are therefore a little bit
noisier to work with from ISLE. They need to maintain the invariant that their
registers really are of the claimed register class, so they need to encapsulate
the inner data. If they exposed the underlying `enum` variants, then anyone
could just change register classes or construct a `GprMem` that holds an XMM
register, defeating the whole point of these newtypes. So when working with
these newtypes from ISLE, we rely on external constructors like `(gpr_to_gpr_mem
my_gpr)` instead of `(GprMem.Gpr my_gpr)`.

A bit of extra lines of code are included to add support for register mapping
for all of these newtypes as well. Ultimately this is all a bit wordier than I'd
hoped it would be when I first started authoring this commit, but I think it is
all worth it nonetheless!

In the process of adding these newtypes, I didn't want to have to update both
the ISLE `extern` type definition of `MInst` and the Rust definition, so I move
the definition fully into ISLE, similar as aarch64.

Finally, this process isn't complete. I've introduced the newtypes here, and
I've made most XMM-using instructions switch from `Reg` to `Xmm`, as well as
register class-converting instructions, but I haven't moved all of the GPR-using
instructions over to the newtypes yet. I figured this commit was big enough as
it was, and I can continue the adoption of these newtypes in follow up commits.

Part of bytecodealliance#3685.
  • Loading branch information
fitzgen committed Feb 3, 2022
1 parent 965c687 commit 8a2f55a
Show file tree
Hide file tree
Showing 21 changed files with 4,244 additions and 3,123 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cranelift/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ serde = { version = "1.0.94", features = ["derive"], optional = true }
bincode = { version = "1.2.1", optional = true }
gimli = { version = "0.26.0", default-features = false, features = ["write"], optional = true }
smallvec = { version = "1.6.1" }
regalloc = { version = "0.0.33" }
regalloc = "0.0.34"
souper-ir = { version = "2.1.0", optional = true }
# It is a goal of the cranelift-codegen crate to have minimal external dependencies.
# Please don't add any unless they are essential to the task of creating binary
Expand Down
2 changes: 2 additions & 0 deletions cranelift/codegen/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,8 @@ fn rebuild_isle(
) -> Result<(), Box<dyn std::error::Error + 'static>> {
use cranelift_isle as isle;

println!("Rebuilding {}", compilation.output.display());

// First, remove the manifest, if any; we will recreate it
// below if the compilation is successful. Ignore error if no
// manifest was present.
Expand Down
5 changes: 0 additions & 5 deletions cranelift/codegen/src/isa/aarch64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -871,15 +871,10 @@

(type BoxCallInfo (primitive BoxCallInfo))
(type BoxCallIndInfo (primitive BoxCallIndInfo))
(type VecMachLabel (primitive VecMachLabel))
(type CondBrKind (primitive CondBrKind))
(type BranchTarget (primitive BranchTarget))
(type BoxJTSequenceInfo (primitive BoxJTSequenceInfo))
(type BoxExternalName (primitive BoxExternalName))
(type CodeOffset (primitive CodeOffset))
(type ExternalName (primitive ExternalName))
(type ValueLabel (primitive ValueLabel))
(type UnwindInst (primitive UnwindInst))

(type ExtendOp extern
(enum
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
src/clif.isle 9ea75a6f790b5c03
src/prelude.isle 2bfcafbef6b29358
src/isa/aarch64/inst.isle 944323ff7d6db098
src/prelude.isle 6aaf8ce0f5a5c2ec
src/isa/aarch64/inst.isle dafd813ba278ce19
src/isa/aarch64/lower.isle 2d2e1e076a0c8a23
Loading

0 comments on commit 8a2f55a

Please sign in to comment.