Evict larger limits first using spill weights

[abrown]

Consider an instruction that could potentially overallocate to a limited range of registers:

Use: v0i limit(0..=1), Use: v0i limit(0..=1), Use: v1i limit(0..=7)

Both uses of v0 must fit in registers 0-1 and v1 should find its home somewhere else in registers 2-7. But fuzzing found a case where this failed. If v1 was allocated first--say to register 0--and one of the uses of v0 allocated to the remaining register--register 1--the final use of v0 had nowhere to go and could not evict either of the previous allocations. This caused ion to fail with TooManyLiveRegs when in fact a solution was possible.

(Why are the identical uses of v0 allocated separately? Can't they use the same register? I'm not entirely sure, but I think ion had split things down to minimal bundles).

The fix in this change is to alter the default spill weights assigned to a minimal bundle. Previously, minimal bundles with a fixed constraint received the highest weight, followed by other minimal bundles and then non-minimal bundles. This reserves weights for limits into that order:

• minimal with fixed constraint
• minimal with limit(0..=0) constraint
• minimal with limit(0..=1) constraint
• ...
• minimal with limit(0..=255) constraint
• minimal, any other constraint
• non-minimal

Doing this allows ion to evict bundles with higher limits (e.g., v1i limit(0..=7) once they become minimal, allowing allocation to continue.

[abrown]

No hurry to merge this just yet; I'm running this in my branch that enables fuzzing of limits: fuzz-limit-constraints-rebased. Thanks to @rahulchaphalkar for helping discover a minimizable test case and @cfallin for talking through a solution!

[abrown]

Also, if it is helpful, I can include an extra commit here with a unit test that runs the exact test case that caused the original problem:

{
  block0(): # succs:[] preds:[]
    inst0: Op ops:[Def@Early: v0i limit(0..=3)] clobber:[]
    inst1: Op ops:[Def: v1i fixed(p25i), Use: v0i fixed(p26i), Use: v0i fixed(p27i)] clobber:[]
    inst2: Op ops:[Def: v2i reg, Use: v0i limit(0..=1), Use: v0i limit(0..=1), Use: v1i limit(0..=7)] clobber:[]
    inst3: Op ops:[Def: v3i reg, Use: v0i fixed(p28i), Use: v0i any] clobber:[]
    inst4: Ret ops:[] clobber:[]
}

[cfallin]

LGTM (no surprise since we pair-programmed this fix) -- thanks a bunch for working on this!

Regarding the unit test for this case -- I guess it depends on whether the fuzzer would cover this (ie the reason we don't have a test suite otherwise, since the fuzzer coverage is so good). This case was originally discovered via fuzzing and the original fuzzbug is now fixed? If so then I think it's fine; otherwise it might be good to throw in an explicit test. Thanks!

[cfallin]

It's been a bit and this change still looks right to me, so I'm going to go ahead and take this out of draft mode and merge. Thanks again!

[abrown]

@cfallin, apologies for not following up with the fuzzing results. I did run fuzzing for this change using fuzz-limit-constraints-rebased and found a fuzz bug that was unrelated to this change. IIRC, the test case did not even have limit constraints at all and my conclusion was that the changes in that branch allowed the fixed constraints to get a little bit too... constrained, causing the "too many live regs" failure. Looking at that derailed me a bit (plus Christmas, jobs, etc.!) so, in conclusion, I would (1) agree that this is still the right change, even if we couldn't properly fuzz it, and (2) alert @rahulchaphalkar that fuzz-limit-constraints-rebased might need some extra eyes on whatever changed with fixed registers (I'll send you the test case).

[abrown]

After looking around I cannot find the fixed-only crash I just mentioned but here is one that fails on the interaction between limit and fixed; I still think the handling of fixed on that branch could use another look:

{
      block0(): # succs:[] preds:[]
        inst0: Op ops:[Def: v0i reg] clobber:[]
        inst1: Op ops:[Def@Early: v1i reg, Use: v0i any, Use: v0i fixed(p35i)] clobber:[]
        inst2: Op ops:[Def@Early: v2i fixed(p0i), Use: v0i any, Use: v0i fixed(p9i), Use: v0i any, Use: v0i any, Use: v0i any, Use: v0i any] clobber:[]
        inst3: Op ops:[Def: v3i limit(0..=7), Use: v0i reg, Use: v0i any, Use: v0i reg, Use: v0i limit(0..=7), Use: v2i limit(0..=7), Use: v2i limit(0..=7), Use: v2i limit(0..=7), Use: v2i limit(0..=15), Use: v2i limit(0..=15)] clobber:[]
        inst4: Op ops:[Def@Early: v4i limit(0..=7), Use: v0i limit(0..=7), Use: v0i limit(0..=7), Use: v0i limit(0..=7), Use: v0i limit(0..=7), Use: v0i limit(0..=7), Use: v0i limit(0..=7), Use: v0i limit(0..=7), Use: v2i limit(0..=15), Use: v1i fixed(p0i)] clobber:[]
        inst5: Op ops:[Def@Early: v5i any] clobber:[]
        inst6: Op ops:[Def@Early: v6i any] clobber:[]
        inst7: Op ops:[Def@Early: v7f any] clobber:[]
        inst8: Op ops:[Def@Early: v8i any] clobber:[]
        inst9: Op ops:[Def@Early: v9v any] clobber:[]
        inst10: Op ops:[Def@Early: v10i any] clobber:[]
        inst11: Op ops:[Def@Early: v11i any] clobber:[]
        inst12: Ret ops:[] clobber:[]
    }