cranelift: Add newtype wrappers for x64 register classes

[fitzgen]

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 enums 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 #3685.

[github-actions]

Subscribe to Label Action

cc @cfallin, @fitzgen

Details

This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "isle"

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

• cfallin: isle
• fitzgen: isle

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

Learn more.

[cfallin]

Thanks for doing all of this -- it's really quite tedious!

I admit to harboring a little doubt as I look at the number of conversions (to_writable_reg, from_writable_reg, etc) and type-tetris this code adds, though this has to be traded off against the correctness benefits. The Writable<T> technique has likely saved us a few times (judged by the several regalloc metadata sort of bugs we've had) and this will probably have similar yields.

I guess a good answer to that is that implicit conversions (#3753) should make almost all of these go away; we should probably prioritize that work (I can take a look at it soon-ish probably, once I'm back from Wasmtime-land).

[cfallin]

Would these constructors be clearer without the _new suffix, I wonder? I'm mostly thinking in terms of symmetry with e.g. instruction constructors and the other utility types -- we have (value_regs ...), not (value_regs_new ...). (Or maybe there's a name conflict from elsewhere I'm missing?)

[fitzgen]

I can do reg_mem_to_xmm_mem which is more similar to the other constructors. Thoughts?

[cfallin]

That seems reasonable to me! I guess as a general principle the x_to_y explicitness is good and is a sort of direct indication that we can register all such constructors as implicit conversions later.

[fitzgen]

I guess a good answer to that is that implicit conversions (#3753) should make almost all of these go away; we should probably prioritize that work (I can take a look at it soon-ish probably, once I'm back from Wasmtime-land).

Yes, I think this the type conversions mechanism will make all of this much better.