Conflicting defs and clobbers lead to panic rather than clean error from allocator

[MaxVerevkin]

regalloc2 v0.12.0

thread 'main' panicked at /home/maxv/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/regalloc2-0.12.0/src/ion/process.rs:1253:17:
Could not allocate minimal bundle, but the allocation problem should be possible to solve

Function which causes a panic:

Debug repr

{
 machine_env: MachineEnv {
 preferred_regs_by_class: [
 [
 PReg(hw = 0, class = Int, index = 0),
 PReg(hw = 1, class = Int, index = 1),
 PReg(hw = 2, class = Int, index = 2),
 ],
 [],
 [],
 ],
 non_preferred_regs_by_class: [
 [],
 [],
 [],
 ],
 scratch_by_class: [
 None,
 None,
 None,
 ],
 fixed_stack_slots: [],
}
 spillslot_size(Int): 1
 spillslot_size(Float): 1
 spillslot_size(Vector): 1
 multi_spillslot_named_by_last_slot: false
 allow_multiple_vreg_defs: false
 block0(): # succs:[] preds:[]
 inst0: op Def: v0i any
 inst1: op Def: v1i fixed(p0i), Use: v0i fixed(p1i), Clobber: p0i, Clobber: p1i, Clobber: p2i
 inst2: ret Use: v1i fixed(p0i)
}

JSON repr

{
 "machine_env": {
 "preferred_regs_by_class": [
 [
 {
 "bits": 0
 },
 {
 "bits": 1
 },
 {
 "bits": 2
 }
 ],
 [],
 []
 ],
 "non_preferred_regs_by_class": [
 [],
 [],
 []
 ],
 "scratch_by_class": [
 null,
 null,
 null
 ],
 "fixed_stack_slots": []
 },
 "entry_block": 0,
 "insts": [
 {
 "op": "Op",
 "operands": [
 {
 "bits": 8388609
 }
 ],
 "clobbers": {
 "bits": [
 0,
 0,
 0,
 0
 ]
 }
 },
 {
 "op": "Op",
 "operands": [
 {
 "bits": 2155872256
 },
 {
 "bits": 2197815297
 }
 ],
 "clobbers": {
 "bits": [
 7,
 0,
 0,
 0
 ]
 }
 },
 {
 "op": "Ret",
 "operands": [
 {
 "bits": 2164260864
 }
 ],
 "clobbers": {
 "bits": [
 0,
 0,
 0,
 0
 ]
 }
 }
 ],
 "blocks": [
 [
 0,
 3
 ]
 ],
 "block_preds": [
 []
 ],
 "block_succs": [
 []
 ],
 "block_params_in": [
 []
 ],
 "block_params_out": [
 []
 ],
 "num_vregs": 2,
 "debug_value_labels": [],
 "spillslot_size": [
 1,
 1,
 1
 ],
 "multi_spillslot_named_by_last_slot": false,
 "allow_multiple_vreg_defs": false
}

[cfallin]

inst1: op Def: v1i fixed(p0i), Use: v0i fixed(p1i), Clobber: p0i, Clobber: p1i, Clobber: p2i

Hi @MaxVerevkin -- this is an invalid input program: it is requesting v0 to be allocated into p1, but p1 is also clobbered. Clobbers mean that nothing can go in that register, even defs.

Where did you get this input program from -- constructed by hand or your own frontend? Or did it come out of fuzzing somehow?

In any case, this is not a bug in the core allocator. It is probably better that we check for this and return a clean error upfront, though, so I'll change the issue title.

[MaxVerevkin]

But the docs say:

Note that it is legal for a register to be both a clobber and an actual def (via pinned vreg or via operand constrained to the reg). This is for convenience: e.g., a call instruction might have a constant clobber set determined by the ABI, but some of those clobbered registers are sometimes return value(s).

After adjusting clobbers to not include the definitions, I'm no longer able to reproduce this panic. Using a to-be-clobbed register is fine, this does not panic:

inst1: op Def: v1i fixed(p0i), Use: v0i fixed(p5i), Clobber: p4i, Clobber: p5i.

But this does:

inst1: op Def: v1i fixed(p0i), Use: v0i fixed(p5i), Clobber: p0i, Clobber: p4i, Clobber: p5i

Where did you get this input program from -- constructed by hand or your own frontend? Or did it come out of fuzzing somehow?

My own frontend, but I'm still learning how to use this library.

It is probably better that we check for this and return a clean error upfront, though, so I'll change the issue title.

Yeah, definitely. Are panics on invalid inputs considered bugs (with validation turned on)? I've found at least one more (assertion error, but the input is more obviously broken).

[cfallin]

Ah, sorry, the docs are out of date then -- this has been the interface for a while now. I'll update that as well.

Ideally we would catch and return clean errors for all invalid input, but we're not quite "panic-clean" yet. In practice regalloc2 is developed in tandem with Cranelift, and the realistic definition of valid input is "exactly what Cranelift generates". Getting beyond that will require more effort to define all of the semantic edge-cases properly (as we see here!). I do want to support other users as much as time allows; sorry for the rough edges!

[MaxVerevkin]

Thanks for the explanation :) And thanks for making this a stand-alone crate usable from other projects!