search

bytecodealliance / regalloc2

A new register allocator
Apache License 2.0
209 stars 38 forks source link

Added support for multiple OperandConstraint::Reuse operands. #180

Open Iizerd opened 1 month ago

Iizerd commented 1 month ago

Previously instructions that have multiple operands specifying the constraint OperandConstraint::Reuse would not work properly. Good examples of this are X86's XCHG and XADD, both of which require two OperandConstraint::Reuses. This PR addresses this issue and fixes it by using a SmallVec<[VReg; 4]> to store all reuse constraints to check, instead of a single Option<VReg>.

I ran 8 instances of ion_checker overnight without fail. Additionally, the fixes have enabled my compiler to correctly emit the aforementioned instructions and it passes my own test suite. I'm not however confident that the ion_checker correctly generates these types of instructions as prior to this change it was still reporting no errors. It also seems to only generate one Def per instruction.

I will be leaving a comment in https://github.com/bytecodealliance/regalloc2/issues/145 as I discovered some interesting things about that as well while looking into this.

As discussed in the other PR(sorry I had to recreate to change source branch) I will look into changing the fuzzer, I will hopefully find time to do this within the coming week or two.

cfallin commented 2 weeks ago

Hi @Iizerd -- I'd still like to see the fuzzing for this before we merge it, to be sure we've got it right -- any update on this?

As discussed in the other PR(sorry I had to recreate to change source branch) I will look into changing the fuzzer, I will hopefully find time to do this within the coming week or two.