Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cranelift: Add newtype wrappers for x64 register classes #3752

Merged
merged 3 commits into from
Feb 3, 2022
Merged

cranelift: Add newtype wrappers for x64 register classes #3752

merged 3 commits into from
Feb 3, 2022

Commits on Feb 3, 2022

  1. ISLE: Add a nodebug type attribute to disable derive(Debug) 

    I need this to move the x64 `Inst` definition into ISLE without losing its
custom `Debug` implementation that prints the assembly for the `Inst`.
    @fitzgen
    fitzgen committed Feb 3, 2022
    Configuration menu
    Copy the full SHA
    e1f4e29 View commit details
    Browse the repository at this point in the history

  2. cranelift: Add newtype wrappers for x64 register classes 

    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.
    @fitzgen
    fitzgen committed Feb 3, 2022
    Configuration menu
    Copy the full SHA
    795b0aa View commit details
    Browse the repository at this point in the history

  3. ISLE: Rename {gpr,xmm}_mem_new constructors to `reg_mem_to_{gpr,xmm… 

    …}_mem`
    @fitzgen
    fitzgen committed Feb 3, 2022
    Configuration menu
    Copy the full SHA
    2c77cf8 View commit details
    Browse the repository at this point in the history