From a401612ac32ecdd2edd4cdbfeba352febc96fd51 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Tue, 7 Nov 2023 14:27:05 +0200 Subject: [PATCH] fix: always create the destination branch argument for the sentinel value 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. --- frontend-wasm/src/ssa.rs | 65 +++++------------------------ tests/integration/expected/fib.hir | 31 +++++++++++--- tests/integration/expected/fib.masm | 45 +++++++++++++++++++- tests/integration/expected/fib.wat | 18 ++++++-- tests/rust-apps/fib/src/lib.rs | 8 ++-- 5 files changed, 98 insertions(+), 69 deletions(-) diff --git a/frontend-wasm/src/ssa.rs b/frontend-wasm/src/ssa.rs index ee769f28..bce273c8 100644 --- a/frontend-wasm/src/ssa.rs +++ b/frontend-wasm/src/ssa.rs @@ -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: /// @@ -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. diff --git a/tests/integration/expected/fib.hir b/tests/integration/expected/fib.hir index ab0d6259..17827022 100644 --- a/tests/integration/expected/fib.hir +++ b/tests/integration/expected/fib.hir @@ -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; } diff --git a/tests/integration/expected/fib.masm b/tests/integration/expected/fib.masm index 28373bb6..f0242ec5 100644 --- a/tests/integration/expected/fib.masm +++ b/tests/integration/expected/fib.masm @@ -667,18 +667,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 diff --git a/tests/integration/expected/fib.wat b/tests/integration/expected/fib.wat index 1ce7cd18..87fdaf1c 100644 --- a/tests/integration/expected/fib.wat +++ b/tests/integration/expected/fib.wat @@ -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) diff --git a/tests/rust-apps/fib/src/lib.rs b/tests/rust-apps/fib/src/lib.rs index eb1fc12a..1b8a8e75 100644 --- a/tests/rust-apps/fib/src/lib.rs +++ b/tests/rust-apps/fib/src/lib.rs @@ -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 }