Skip to content

Commit

Permalink
[lsra]: Fix nested live ranges due to finishLoop
Browse files Browse the repository at this point in the history
In certain edge cases, `finishLoop` would not insert `LsraEnd`s for
active variables resulting in nested live ranges which manifested as an
infinite loop in the register allocator (see discussion here [1]).

This commit fixes this bug by keeping track of whether a variable will
be made active in the future or not (i.e. if it will have a live range).
If a variable will have a live range in the future, then `finishLoop`
will make sure to insert `LsraEnd`s for them to avoid the above bug.

[1]: titzer#308
  • Loading branch information
k-sareen committed Dec 12, 2024
1 parent bc54761 commit 131ea3e
Show file tree
Hide file tree
Showing 12 changed files with 430 additions and 10 deletions.
93 changes: 84 additions & 9 deletions aeneas/src/mach/LinearScanRegAlloc.v3
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ class LinearScanRegAlloc {
}
def recordLive(index: int) {
for (l = activeList; l != null; l = l.next) {
if (l == l.next) {
gen.mach.prog.ERROR.fail(
Strings.format2("VReg %d from SSAInstr %d points to itself!", l.varNum, l.ssa.uid));
}
livemap[index, l.varNum] = true;
}
}
Expand All @@ -111,6 +115,12 @@ class LinearScanRegAlloc {
// add to the active list
// if (Debug.PARANOID && (vreg.next != null || vreg.prev != null)) fail("var %d should not be in list", vreg.varNum);
var prev = activeList;
if (prev == vreg) {
var b = StringBuilder.new();
b.put3("Making a circular linked list with VRegs %d and %d from SSAInstrs %d and ",
prev.varNum, vreg.varNum, prev.ssa.uid).put1("%d respectively", vreg.ssa.uid);
gen.mach.prog.ERROR.fail(b.toString());
}
vreg.next = prev;
activeList = vreg;
if (prev != null) prev.prev = vreg;
Expand All @@ -121,6 +131,12 @@ class LinearScanRegAlloc {
var vreg = p.vreg;
if (vreg == null) return; // nothing to do
var pos = p.pos, p = vreg.prev, n = vreg.next;
if (p == n && p != null) {
var b = StringBuilder.new();
b.put3("Have a circular linked list with VRegs %d and %d from SSAInstrs %d and ",
p.varNum, vreg.varNum, p.ssa.uid).put1("%d respectively", vreg.ssa.uid);
gen.mach.prog.ERROR.fail(b.toString());
}
if (p != null) p.next = n;
if (n != null) n.prev = p;
vreg.prev = null;
Expand Down Expand Up @@ -448,7 +464,10 @@ class IntervalBuilder(gen: OldCodeGen, regSet: MachRegSet) {
for (i < brow.length) {
var vnum = i * 32, bb = brow[i], pb = prow[i];
for (bits = bb & (-1 ^ pb); bits != 0; bits = bits >>> 1) {
if ((bits & 1) != 0) insertHead(LsraStart.new(b.start * 2, vars[vnum]));
if ((bits & 1) != 0) {
insertHead(LsraStart.new(b.start * 2, vars[vnum]));
vars[vnum].activeInFuture = true;
}
vnum++;
}
}
Expand All @@ -457,7 +476,10 @@ class IntervalBuilder(gen: OldCodeGen, regSet: MachRegSet) {
for (i < brow.length) {
var vnum = i * 32, bb = brow[i], pb = prow[i];
for (bits = (-1 ^ bb) & pb; bits != 0; bits = bits >>> 1) {
if ((bits & 1) != 0) insertHead(LsraEnd.new(b.start * 2, vars[vnum]));
if ((bits & 1) != 0) {
insertHead(LsraEnd.new(b.start * 2, vars[vnum]));
vars[vnum].activeInFuture = false;
}
vnum++;
}
}
Expand All @@ -481,7 +503,10 @@ class IntervalBuilder(gen: OldCodeGen, regSet: MachRegSet) {
var vnum = u >>> gen.VAR_SHIFT, vreg = vars[vnum];
if (usePoint == null) insertHead(usePoint = LsraUse.new(pos, i));
if (!vreg.isConst()) {
if (!livein.set(blindex, vnum)) insertHead(LsraEnd.new(pos, vreg));
if (!livein.set(blindex, vnum)) {
insertHead(LsraEnd.new(pos, vreg));
vreg.activeInFuture = false;
}
if (vreg.hint == 0 && regSet.isReg(fixed)) vreg.hint = byte.!(fixed);
if (pos > vreg.endPos) vreg.endPos = pos;
}
Expand Down Expand Up @@ -527,8 +552,18 @@ class IntervalBuilder(gen: OldCodeGen, regSet: MachRegSet) {
while (l != null && l.pos < loopEndPos) {
if (l.vreg != null && livein[0, l.vreg.varNum]) {
// remove all starts and ends within the loop
if (LsraStart.?(l)) { l = removePoint(l); continue; }
if (LsraEnd.?(l)) { l = removePoint(l); continue; }
if (LsraStart.?(l)) {
// Since we're removing an LsraStart, the VReg will not be active in the future anymore
l.vreg.activeInFuture = false;
l = removePoint(l);
continue;
}
if (LsraEnd.?(l)) {
// Since we're removing an LsraEnd, the VReg will be active in the future
l.vreg.activeInFuture = true;
l = removePoint(l);
continue;
}
}
p = l;
l = l.next;
Expand All @@ -554,29 +589,60 @@ class IntervalBuilder(gen: OldCodeGen, regSet: MachRegSet) {
p = l;
l = l.next;
}

if (insertPoint == null) {
// if there are no points after the loop end, add a dummy one
insertPoint = LsraEnd.new(-1, null);
insertPoint.prev = p;
if (p != null) p.next = insertPoint;
}

var vregsActiveInFuture = Vector<VReg>.new();
// if the last block of the loop is a predecessor of the block after
// the loop, then a correct live range hole was already created
var order = gen.blocks.order, last = order[b.loop.end - 1].block;
if (b.loop.end < order.length) {
var next = order[b.loop.end].block;
var isPred = false;
for (s in last.succs()) {
if (s.dest == next) return;
if (s.dest == next) {
isPred = true;
break;
}
}

if (isPred) {
// XXX: If any variable will made active in the future, we need to
// ensure we insert LsraEnds to create live range holes
for (v < vars.length) {
if (livein[0, v] && vars[v].activeInFuture) {
vregsActiveInFuture.put(vars[v]);
}
}
if (vregsActiveInFuture.length == 0) {
return;
}
}
}

// create a live range hole by inserting ends for all remaining variables
livein.row(0).apply(insertEndVarAtLoopEnd, (loopEndPos, insertPoint));
if (vregsActiveInFuture.length > 0) {
// XXX: Use a Vector instead of calling `livein.row(0).apply` since there
// may be cases where only certain variables are made active in the
// future, instead of everything livein to the loop
for (vreg in vregsActiveInFuture.extract()) {
insertEndVarAtLoopEnd(vreg.varNum, (loopEndPos, insertPoint));
}
} else {
livein.row(0).apply(insertEndVarAtLoopEnd, (loopEndPos, insertPoint));
}
// remove dummy point
if (insertPoint.pos == -1) removePoint(insertPoint);
}
def insertEndVarAtLoopEnd(vnum: int, t: (int, LsraPoint)) {
var vreg = vars[vnum], loopEndPos = t.0, insertPoint = t.1;
insertBefore(LsraEnd.new(loopEndPos, vreg), insertPoint);
vreg.activeInFuture = false;
if (loopEndPos > vreg.endPos) vreg.endPos = loopEndPos;
}
def insertHead(n: LsraPoint) {
Expand Down Expand Up @@ -624,6 +690,7 @@ class IntervalPrinter(
var state = Array<byte>.new(1 + gen.vars.length);
var live = Array<byte>.new(1 + gen.vars.length);
var map = Array<int>.new(gen.vars.length);
var sb = StringBuilder.new();
var vnum: int;

new() {
Expand Down Expand Up @@ -724,8 +791,16 @@ class IntervalPrinter(
def pointType(l: LsraPoint) -> string {
if (LsraStart.?(l)) return ("start");
if (LsraEnd.?(l)) return ("end");
if (LsraLive.?(l)) return ("live");
if (LsraKill.?(l)) return ("kill");
if (LsraLive.?(l)) {
sb.reset();
sb.put1("live idx %d", LsraLive.!(l).index);
return (sb.toString());
}
if (LsraKill.?(l)) {
sb.reset();
sb.put1("kill regset 0x%x", LsraKill.!(l).regset);
return (sb.toString());
}
if (LsraUse.?(l)) return ("use");
if (LsraDef.?(l)) return ("def");
return "unknown";
Expand Down
1 change: 1 addition & 0 deletions aeneas/src/mach/MachBackend.v3
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class VReg {
var endPos: int; // last live position
var notSpillable: bool; // true if this var cannot be spill anymore
var reloadFrom: VReg; // var that this var is spilled from
var activeInFuture: bool; // Will the VReg be made active (i.e. starting a new live range) in the future?
// -- state for stackifier -----------------------------
var usage = Usage.NONE;
var parmoveState: int;
Expand Down
2 changes: 1 addition & 1 deletion aeneas/src/main/Version.v3
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@

// Updated by VCS scripts. DO NOT EDIT.
component Version {
def version: string = "III-7.1775";
def version: string = "III-7.1777";
var buildData: string;
}
70 changes: 70 additions & 0 deletions test/core/zjson00.v3
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//@execute 0=true
type JsonValue {
case String(v: string);
case Number(v: int); // TODO: float
case Bool(v: bool);
case Null;
case JArray(v: Array<JsonValue>);
case JObject(v: HashMap<string, JsonValue>);
}

class Vector<T> {
def put(v: T) { }
def extract() -> Array<T> { return null; }
}
class HashMap<K, V> {
}

def ERR_RET = JsonValue.Null;
class JsonParser {
var ok: bool;
var state: int;

def parse_value() -> JsonValue {
match (state++) {
0 => return ERR_RET;
1 => return parse_string();
2 => return parse_number();
3 => return JsonValue.Null;
4 => return JsonValue.Bool(true);
5 => return JsonValue.Bool(false);
6 => return parse_array();
7 => return parse_object();
}
return ERR_RET;
}
def parse_string() -> JsonValue {
return JsonValue.String(if(state++ == 8, "", "a"));
}
def parse_number() -> JsonValue {
return JsonValue.Number(state++);
}
def parse_object() -> JsonValue {
return JsonValue.JObject(null);
}
def parse_array() -> JsonValue {
var vals = Vector<JsonValue>.new();
if (req1('[') == -1) return ERR_RET;
vals.put(parse_value());
if (!ok) return ERR_RET;
while (opt1(',') != -1) {
vals.put(parse_value());
if (!ok) return ERR_RET;
}
if (req1(']') == -1) return ERR_RET;
return JsonValue.JArray(vals.extract());
}
def req1(ch: byte) -> int {
ok = ch == '1';
return if(ch == '0', 1, -1);
}
def opt1(ch: byte) -> int {
ok = ch == '2';
return -1;
}
}

def main(a: int) -> bool {
var x = JsonParser.new();
return x.parse_array() == ERR_RET;
}
70 changes: 70 additions & 0 deletions test/core/zjson01.v3
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//@execute 0=true; 1=false; 2=false
type JsonValue {
case String(v: string);
case Number(v: int); // TODO: float
case Bool(v: bool);
case Null;
case JArray(v: Array<JsonValue>);
case JObject(v: HashMap<string, JsonValue>);
}

class Vector<T> {
def put(v: T) { }
def extract() -> Array<T> { return null; }
}
class HashMap<K, V> {
}

def ERR_RET = JsonValue.Null;
class JsonParser {
var ok: bool;
var state: int;

def parse_value() -> JsonValue {
match (state++) {
0 => return ERR_RET;
1 => return parse_string();
2 => return parse_number();
3 => return JsonValue.Null;
4 => return JsonValue.Bool(true);
5 => return JsonValue.Bool(false);
6 => return parse_array();
7 => return parse_object();
}
return ERR_RET;
}
def parse_string() -> JsonValue {
return JsonValue.String(if(state++ == 8, "", "a"));
}
def parse_number() -> JsonValue {
return JsonValue.Number(state++);
}
def parse_object() -> JsonValue {
return JsonValue.JObject(null);
}
def parse_array() -> JsonValue {
var vals = Vector<JsonValue>.new();
if (req1('[') == -1) return ERR_RET;
vals.put(parse_value());
if (!ok) return ERR_RET;
while (opt1(',') != -1) {
vals.put(parse_value());
if (!ok) return ERR_RET;
}
if (req1(']') == -1) return ERR_RET;
return JsonValue.JArray(vals.extract());
}
def req1(ch: byte) -> int {
return 1;
}
def opt1(ch: byte) -> int {
return -1;
}
}

def main(a: int) -> bool {
var x = JsonParser.new();
x.state = a;
x.ok = a > 0;
return x.parse_array() == ERR_RET;
}
51 changes: 51 additions & 0 deletions test/core/zjson02.v3
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//@execute 0=true; 1=false; 2=false
type JsonValue {
case String(v: string);
case Number(v: int); // TODO: float
case Bool(v: bool);
case Null;
case JArray(v: Array<JsonValue>);
case JObject(v: HashMap<string, JsonValue>);
}

class Vector<T> {
def put(v: T) { }
def extract() -> Array<T> { return null; }
}
class HashMap<K, V> {
}

def ERR_RET = JsonValue.Null;
class JsonParser {
var ok: bool;
var state: int;

def parse_value() -> JsonValue {
return JsonValue.String(if(state++ == 8, "", "a"));
}
def parse_array() -> JsonValue {
var vals = Vector<JsonValue>.new();
if (req1() == -1) return ERR_RET;
vals.put(parse_value());
if (!ok) return ERR_RET;
while (opt1(',') != -1) {
vals.put(parse_value());
if (!ok) return ERR_RET;
}
if (req1() == -1) return ERR_RET;
return JsonValue.JArray(vals.extract());
}
def req1() -> int {
return 1;
}
def opt1(ch: byte) -> int {
return -1;
}
}

def main(a: int) -> bool {
var x = JsonParser.new();
x.state = a;
x.ok = a > 0;
return x.parse_array() == ERR_RET;
}
Loading

0 comments on commit 131ea3e

Please sign in to comment.