Improve the variable ordering used by the coloring constraint solver.

The fuzzer bugs #219 and #227 are both cases where the register
allocator coloring pass "runs out of registers". What's really happening
is that the constraint solver failed to find a solution, even when one
existed.

Suppose we have three solver variables:

    v0(GPR, out, global)
    v1(GPR, in)
    v2(GPR, in, out)

And suppose registers %r0 and %r1 are available on both input and output
sides of the instruction, but only %r1 is available for global outputs.
A valid solution would be:

    v0 -> %r1
    v1 -> %r1
    v2 -> %r0

However, the solver would pick registers for the three values in
numerical order because v1 and v2 have the same domain size (=2). This
would assign v1 -> %r0 and then fail to find a free register for v2.

Fix this by prioritizing in+out variables over single-sided variables
even when their domains are equal. This means the v2 gets assigned a
register before v1, and it gets a chance to pick a register that is
still available on both in and out sides.

Also try to avoid depending on value numbers in the solver. These bugs
were hard to reproduce because a test case invariably would have
different value numbers, causing the solver to order its variables
differently and succeed. Throw in the previous solution and original
register assignments as tie breakers which are stable and not dependent
on value numbers.

This is still not a substitute for a proper solver search algorithm that
we will probably have to write eventually.

Fixes #219
Fixes #227
This commit is contained in:
Jakob Stoklund Olesen
2018-01-19 13:31:26 -08:00
parent 7826fce44f
commit 1bbc529ef9
2 changed files with 132 additions and 2 deletions

View File

@@ -537,6 +537,8 @@ impl Solver {
self.regs_in = regs.clone();
// Used for tracking fixed input assignments while `!inputs_done`:
self.regs_out = AllocatableSet::new();
self.moves.clear();
self.fills.clear();
}
/// Add a fixed input reassignment of `value`.
@@ -891,8 +893,35 @@ impl Solver {
v.domain = cmp::min(d, u16::MAX as usize) as u16;
}
// Solve for vars with small domains first to increase the chance of finding a solution.
// Use the value number as a tie breaker to get a stable sort.
self.vars.sort_unstable_by_key(|v| (v.domain, v.value));
//
// Also consider this case:
//
// v0: out, global
// v1: in
// v2: in+out
//
// If only %r0 and %r1 are available, the global constraint may cause us to assign:
//
// v0 -> %r1
// v1 -> %r0
// v2 -> !
//
// Usually in+out variables will have a smaller domain, but in the above case the domain
// size is the same, so we also prioritize in+out variables.
//
// Include the reversed previous solution for this variable partly as a stable tie breaker,
// partly to shake things up on a second attempt.
//
// Use the `from` register and value number as a tie breaker to get a stable sort.
self.vars.sort_unstable_by_key(|v| {
(
v.domain,
!(v.is_input && v.is_output),
!v.solution,
v.from.unwrap_or(0),
v.value,
)
});
dbg!("real_solve for {}", self);
self.find_solution(global_regs)