diff --git a/crates/nargo_cli/tests/execution_success/slice_dynamic_index/src/main.nr b/crates/nargo_cli/tests/execution_success/slice_dynamic_index/src/main.nr index 48229a0ced3..de5b4caef29 100644 --- a/crates/nargo_cli/tests/execution_success/slice_dynamic_index/src/main.nr +++ b/crates/nargo_cli/tests/execution_success/slice_dynamic_index/src/main.nr @@ -20,6 +20,7 @@ fn regression_dynamic_slice_index(x: Field, y: Field) { dynamic_slice_merge_if(slice, x); dynamic_slice_merge_else(slice, x); + dynamic_slice_merge_two_ifs(slice, x); } fn dynamic_slice_index_set_if(mut slice: [Field], x: Field, y: Field) { @@ -164,6 +165,35 @@ fn dynamic_slice_merge_else(mut slice: [Field], x: Field) { assert(slice[slice.len() - 1] == 20); } +fn dynamic_slice_merge_two_ifs(mut slice: [Field], x: Field) { + if x as u32 > 10 { + assert(slice[x] == 0); + slice[x] = 2; + } else { + assert(slice[x] == 4); + slice[x] = slice[x] - 2; + slice = slice.push_back(10); + } + + assert(slice.len() == 6); + assert(slice[slice.len() - 1] == 10); + + if x == 20 { + slice = slice.push_back(20); + } else { + slice = slice.push_back(15); + } + // TODO(#2599): Breaks if the push back happens without the else case + // slice = slice.push_back(15); + + assert(slice.len() == 7); + assert(slice[slice.len() - 1] == 15); + + slice = slice.push_back(20); + assert(slice.len() == 8); + assert(slice[slice.len() - 1] == 20); +} + fn dynamic_slice_index_set_nested_if_else_else(mut slice: [Field], x: Field, y: Field) { assert(slice[x] == 4); assert(slice[y] == 1); diff --git a/crates/nargo_cli/tests/execution_success/slices/src/main.nr b/crates/nargo_cli/tests/execution_success/slices/src/main.nr index 26cf173a253..8fbe14bfea3 100644 --- a/crates/nargo_cli/tests/execution_success/slices/src/main.nr +++ b/crates/nargo_cli/tests/execution_success/slices/src/main.nr @@ -102,6 +102,18 @@ fn merge_slices_if(x: Field, y: Field) { let slice = merge_slices_mutate_in_loop(x, y); assert(slice[6] == 4); assert(slice.len() == 7); + + let slice = merge_slices_mutate_two_ifs(x, y); + assert(slice.len() == 6); + assert(slice[3] == 5); + assert(slice[4] == 15); + assert(slice[5] == 30); + + let slice = merge_slices_mutate_between_ifs(x, y); + assert(slice.len() == 6); + assert(slice[3] == 5); + assert(slice[4] == 30); + assert(slice[5] == 15); } fn merge_slices_else(x: Field) { @@ -156,4 +168,46 @@ fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { slice = slice.push_back(x); } slice -} \ No newline at end of file +} + +fn merge_slices_mutate_two_ifs(x: Field, y: Field) -> [Field] { + let mut slice = [0; 2]; + if x != y { + slice = slice.push_back(y); + slice = slice.push_back(x); + } else { + slice = slice.push_back(x); + } + if x == 20 { + slice = slice.push_back(20); + } else { + slice = slice.push_back(15); + } + // TODO(#2599): Breaks if the push back happens without the else case + // slice = slice.push_back(15); + slice = slice.push_back(30); + + slice +} + +fn merge_slices_mutate_between_ifs(x: Field, y: Field) -> [Field] { + let mut slice = [0; 2]; + if x != y { + slice = slice.push_back(y); + slice = slice.push_back(x); + } else { + slice = slice.push_back(x); + } + + slice = slice.push_back(30); + + if x == 20 { + slice = slice.push_back(20); + } else { + slice = slice.push_back(15); + } + // TODO(#2599): Breaks if the push back happens without the else case + // slice = slice.push_back(15); + + slice +} diff --git a/crates/noirc_evaluator/src/ssa/ir/instruction.rs b/crates/noirc_evaluator/src/ssa/ir/instruction.rs index feca2f56046..1dd2368b1a0 100644 --- a/crates/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa/ir/instruction.rs @@ -171,7 +171,7 @@ pub(crate) enum Instruction { /// Creates a new array with the new value at the given index. All other elements are identical /// to those in the given array. This will not modify the original array. /// - /// An optional length can be provided to enabling handling of dynamic slice indices + /// An optional length can be provided to enable handling of dynamic slice indices. ArraySet { array: ValueId, index: ValueId, value: ValueId, length: Option }, } diff --git a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs index dbc358c5f19..42d0aa0a4e4 100644 --- a/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/crates/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -5,18 +5,19 @@ use iter_extended::vecmap; use num_bigint::BigUint; use crate::ssa::ir::{ + basic_block::BasicBlockId, dfg::DataFlowGraph, instruction::Intrinsic, map::Id, types::Type, - value::{Value, ValueId}, basic_block::BasicBlockId, + value::{Value, ValueId}, }; use super::{Binary, BinaryOp, Endian, Instruction, SimplifyResult}; /// Try to simplify this call instruction. If the instruction can be simplified to a known value, /// that value is returned. Otherwise None is returned. -/// +/// /// The `block` parameter indicates the block any new instructions that are part of a call's /// simplification will be inserted into. For example, all slice intrinsics require updates /// to the slice length, which requires inserting a binary instruction. This update instruction @@ -232,7 +233,12 @@ pub(super) fn simplify_call( /// The binary operation performed on the slice length is always an addition or subtraction of `1`. /// This is because the slice length holds the user length (length as displayed by a `.len()` call), /// and not a flattened length used internally to represent arrays of tuples. -fn update_slice_length(slice_len: ValueId, dfg: &mut DataFlowGraph, operator: BinaryOp, block: BasicBlockId) -> ValueId { +fn update_slice_length( + slice_len: ValueId, + dfg: &mut DataFlowGraph, + operator: BinaryOp, + block: BasicBlockId, +) -> ValueId { let one = dfg.make_constant(FieldElement::one(), Type::field()); let instruction = Instruction::Binary(Binary { lhs: slice_len, operator, rhs: one }); let call_stack = dfg.get_value_call_stack(slice_len); diff --git a/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 2b9991844c8..d57c2cc7933 100644 --- a/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -489,33 +489,36 @@ impl<'f> Context<'f> { let value = &self.inserter.function.dfg[value_id]; match value { Value::Array { array, .. } => array.len(), - Value::NumericConstant { constant, .. } => constant.to_u128() as usize, Value::Instruction { instruction: instruction_id, .. } => { let instruction = &self.inserter.function.dfg[*instruction_id]; match instruction { - Instruction::ArraySet { length, .. } => { - let length = length.expect("ICE: array set on a slice must have a length"); - self.get_slice_length(length) - } + Instruction::ArraySet { array, .. } => self.get_slice_length(*array), Instruction::Load { address } => { - let context_store = self - .outer_block_stores - .get(address) - .expect("ICE: load in merger should have store from outer block"); + let context_store = if let Some(store) = self.store_values.get(address) { + store + } else { + self.outer_block_stores + .get(address) + .expect("ICE: load in merger should have store from outer block") + }; self.get_slice_length(context_store.new_value) } Instruction::Call { func, arguments } => { let func = &self.inserter.function.dfg[*func]; - let length = arguments[0]; + let slice_contents = arguments[1]; match func { Value::Intrinsic(intrinsic) => match intrinsic { Intrinsic::SlicePushBack | Intrinsic::SlicePushFront - | Intrinsic::SliceInsert => self.get_slice_length(length) + 1, + | Intrinsic::SliceInsert => { + self.get_slice_length(slice_contents) + 1 + } Intrinsic::SlicePopBack | Intrinsic::SlicePopFront - | Intrinsic::SliceRemove => self.get_slice_length(length) - 1, + | Intrinsic::SliceRemove => { + self.get_slice_length(slice_contents) - 1 + } _ => { unreachable!("ICE: Intrinsic not supported, got {intrinsic:?}") }