Skip to content

Commit

Permalink
Modify brili to properly execute phi instructions
Browse files Browse the repository at this point in the history
This fixes issue 330: sampsyo#330

The approach (copy the environment at the end of the last basic block)
was inspired/copied from the comment on the Bril issue:
sampsyo#330 (comment)
  • Loading branch information
tylerhou committed Oct 17, 2024
1 parent 05924c1 commit 349ba2f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 10 deletions.
31 changes: 21 additions & 10 deletions brili.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,15 @@ type State = {
// For profiling: a total count of the number of instructions executed.
icount: bigint,

// For SSA (phi-node) execution: keep track of recently-seen labels.j
// For SSA (phi-node) execution: keep track of recently-seen labels.
curlabel: string | null,
lastlabel: string | null,
lastblock: null | {
label: string,
// The environment at the end of the last basic block. To evaluate the RHS
// of phi nodes, we lookup values in this map to avoid the "swap problem"
// (https://github.com/sampsyo/bril/issues/330)
endenv: Env,
},

// For speculation: the state at the point where speculation began.
specparent: State | null,
Expand Down Expand Up @@ -363,7 +369,7 @@ function evalCall(instr: bril.Operation, state: State): Action {
heap: state.heap,
funcs: state.funcs,
icount: state.icount,
lastlabel: null,
lastblock: null,
curlabel: null,
specparent: null, // Speculation not allowed.
}
Expand Down Expand Up @@ -681,10 +687,10 @@ function evalInstr(instr: bril.Instruction, state: State): Action {
if (labels.length != args.length) {
throw error(`phi node has unequal numbers of labels and args`);
}
if (!state.lastlabel) {
if (state.lastblock === null) {
throw error(`phi node executed with no last label`);
}
let idx = labels.indexOf(state.lastlabel);
let idx = labels.indexOf(state.lastblock.label);
if (idx === -1) {
// Last label not handled. Leave uninitialized.
state.env.delete(instr.dest);
Expand All @@ -694,7 +700,7 @@ function evalInstr(instr: bril.Instruction, state: State): Action {
throw error(`phi node needed at least ${idx+1} arguments`);
}
let src = instr.args[idx];
let val = state.env.get(src);
let val = state.lastblock.endenv.get(src);
if (val === undefined) {
state.env.delete(instr.dest);
} else {
Expand Down Expand Up @@ -811,7 +817,7 @@ function evalFunc(func: bril.Function, state: State): Value | null {
// count "aborted" instructions.
Object.assign(state, {
env: state.specparent.env,
lastlabel: state.specparent.lastlabel,
lastblock: state.specparent.lastblock,
curlabel: state.specparent.curlabel,
specparent: state.specparent.specparent,
});
Expand Down Expand Up @@ -839,8 +845,13 @@ function evalFunc(func: bril.Function, state: State): Value | null {
}
}
} else if ('label' in line) {
// Update CFG tracking for SSA phi nodes.
state.lastlabel = state.curlabel;
if (state.curlabel !== null) {
// Update CFG tracking for SSA phi nodes.
state.lastblock = {
label: state.curlabel,
endenv: new Map(state.env),
};
}
state.curlabel = line.label;
}
}
Expand Down Expand Up @@ -943,7 +954,7 @@ function evalProg(prog: bril.Program) {
heap,
env: newEnv,
icount: BigInt(0),
lastlabel: null,
lastblock: null,
curlabel: null,
specparent: null,
}
Expand Down
21 changes: 21 additions & 0 deletions test/interp/ssa/ssa-swap.bril
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
@main {
i: int = const 5;
one: int = const 1;
zero: int = const 0;

.l0:
x0: int = const 0;
y0: int = const 1;
jmp .l1;

.l1:
x1: int = phi .l0 x0 .l1 y1;
y1: int = phi .l0 y0 .l1 x1;
print x1 y1;

cond: bool = gt i zero;
i: int = sub i one;
br cond .l1 .end;

.end:
}
6 changes: 6 additions & 0 deletions test/interp/ssa/ssa-swap.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
0 1
1 0
0 1
1 0
0 1
1 0

0 comments on commit 349ba2f

Please sign in to comment.