regalloc2 icon indicating copy to clipboard operation
regalloc2 copied to clipboard

Added support for multiple OperandConstraint::Reuse operands.

Open Iizerd opened this issue 1 year ago • 1 comments

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.

Iizerd avatar Jul 25 '24 21:07 Iizerd

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.

cfallin avatar Sep 03 '24 16:09 cfallin