Skip to content

Commit

Permalink
fix: always create the destination branch argument for the sentinel v…
Browse files Browse the repository at this point in the history
…alue

when looking for the value definition in the predecessors. #51

This removes the remnants of the previously removed `ValueData::Alias` variant.
It does not matter if we find the definition in the predecessors since we cannot
swap all sentinel value uses with it. So this fix always makes the sentinel value into a
block argument.
  • Loading branch information
greenhat committed Nov 16, 2023
1 parent 20a62a2 commit 46d0dbc
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 69 deletions.
65 changes: 11 additions & 54 deletions frontend-wasm/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,6 @@ enum Call {
FinishPredecessorsLookup(Value, Block),
}

/// Emit instructions to produce a zero value in the given type.
fn emit_zero(_ty: &Type, //, mut cur: FuncCursor
) -> Value {
todo!("emit zero value at the beginning of a block")
}

/// The following methods are the API of the SSA builder. Here is how it should be used when
/// translating to Miden IR:
///
Expand Down Expand Up @@ -461,55 +455,18 @@ impl SSABuilder {
let num_predecessors = self.predecessors(dest_block).len();
// When this `Drain` is dropped, these elements will get truncated.
let results = self.results.drain(self.results.len() - num_predecessors..);

let pred_val = {
let mut iter = results.as_slice().iter().filter(|&val| val != &sentinel);
if let Some(val) = iter.next() {
// This variable has at least one non-temporary definition. If they're all the same
// value, we can remove the block parameter and reference that value instead.
if iter.all(|other| other == val) {
Some(*val)
} else {
None
}
} else {
// The variable is used but never defined before. This is an irregularity in the
// code, but rather than throwing an error we silently initialize the variable to
// 0. This will have no effect since this situation happens in unreachable code.
if !dfg.is_block_inserted(dest_block) {
dfg.append_block(dest_block);
}
self.side_effects
.instructions_added_to_blocks
.push(dest_block);
let zero = emit_zero(
dfg.value_type(sentinel),
// FuncCursor::new(func).at_first_insertion_point(dest_block),
);
Some(zero)
}
};

if let Some(pred_val) = pred_val {
// Here all the predecessors use a single value to represent our variable
// so we don't need to have it as a block argument.
dfg.remove_block_param(sentinel);
pred_val
} else {
// There is disagreement in the predecessors on which value to use so we have
// to keep the block argument.
let mut preds = self.ssa_blocks[dest_block].predecessors;
for (idx, &val) in results.as_slice().iter().enumerate() {
let pred = preds.get_mut(idx, &mut self.inst_pool).unwrap();
let branch = *pred;
assert!(
dfg.insts[branch].opcode().is_branch(),
"you have declared a non-branch instruction as a predecessor to a block!"
);
dfg.append_branch_destination_argument(branch, dest_block, val);
}
sentinel
// Keep the block argument.
let mut preds = self.ssa_blocks[dest_block].predecessors;
for (idx, &val) in results.as_slice().iter().enumerate() {
let pred = preds.get_mut(idx, &mut self.inst_pool).unwrap();
let branch = *pred;
assert!(
dfg.insts[branch].opcode().is_branch(),
"you have declared a non-branch instruction as a predecessor to a block!"
);
dfg.append_branch_destination_argument(branch, dest_block, val);
}
sentinel
}

/// Returns the list of `Block`s that have been declared as predecessors of the argument.
Expand Down
31 changes: 25 additions & 6 deletions tests/integration/expected/fib.hir
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,29 @@ global external @gv2 : i32 = $0 { id = 2 };

pub fn fib(i32) -> i32 {
block0(v0: i32):
v2 = const.i32 1000 : i32;
v3 = cast v0 : u32;
v4 = cast v2 : u32;
v5 = lt v3, v4 : i1;
v6 = cast v5 : i32;
ret v6;
br block2(v0);

block1(v1: i32):

block2(v3: i32):
v4 = const.i32 3 : i32;
v5 = cast v3 : u32;
v6 = cast v4 : u32;
v7 = gt v5, v6 : i1;
v8 = cast v7 : i32;
v9 = neq v8, 0 : i1;
condbr v9, block4, block5;

block3(v2: i32):

block4:
v10 = const.i32 3 : i32;
v11 = cast v3 : u32;
v12 = cast v10 : u32;
v13 = div.checked v11, v12 : u32;
v14 = cast v13 : i32;
br block2(v14);

block5:
ret v3;
}
45 changes: 43 additions & 2 deletions tests/integration/expected/fib.masm
Original file line number Diff line number Diff line change
Expand Up @@ -602,18 +602,59 @@ export.load_dw
end

export.fib
dup.0
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
push.1000
push.3
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
u32.lt.checked
u32.gt.checked
u32.neq.0
push.1
while.true
if.true
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
push.3
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
u32.div.checked
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
dup.0
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
push.3
dup.0
push.2147483648
u32.and
eq.2147483648
assertz
u32.gt.checked
u32.neq.0
push.1
else
push.0
end
end

end
begin
Expand Down
18 changes: 15 additions & 3 deletions tests/integration/expected/fib.wat
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
(module
(type (;0;) (func (param i32) (result i32)))
(func $fib (;0;) (type 0) (param i32) (result i32)
local.get 0
i32.const 1000
i32.lt_u
loop (result i32) ;; label = @1
block ;; label = @2
local.get 0
i32.const 3
i32.gt_u
br_if 0 (;@2;)
local.get 0
return
end
local.get 0
i32.const 3
i32.div_u
local.set 0
br 0 (;@1;)
end
)
(table (;0;) 1 1 funcref)
(memory (;0;) 16)
Expand Down
8 changes: 4 additions & 4 deletions tests/rust-apps/fib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ pub fn fib(n: u32) -> u32 {
// }
// a

if n < 1000 {
1
} else {
0
let mut a = n;
while a > 3 {
a = a / 3;
}
a
}

0 comments on commit 46d0dbc

Please sign in to comment.