diff --git a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs index 267a2c105f2..e7d765949b9 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs @@ -2,7 +2,7 @@ use crate::ssa::{ ir::{ basic_block::BasicBlockId, dfg::DataFlowGraph, - function::Function, + function::{Function, RuntimeType}, instruction::{Instruction, InstructionId, TerminatorInstruction}, types::Type::{Array, Slice}, value::ValueId, @@ -34,14 +34,17 @@ impl Function { let mut array_to_last_use = HashMap::default(); let mut instructions_to_update = HashSet::default(); let mut arrays_from_load = HashSet::default(); + let mut inner_nested_arrays = HashMap::default(); for block in reachable_blocks.iter() { analyze_last_uses( &self.dfg, *block, + matches!(self.runtime(), RuntimeType::Brillig), &mut array_to_last_use, &mut instructions_to_update, &mut arrays_from_load, + &mut inner_nested_arrays, ); } for block in reachable_blocks { @@ -55,9 +58,11 @@ impl Function { fn analyze_last_uses( dfg: &DataFlowGraph, block_id: BasicBlockId, + is_brillig_func: bool, array_to_last_use: &mut HashMap, instructions_that_can_be_made_mutable: &mut HashSet, arrays_from_load: &mut HashSet, + inner_nested_arrays: &mut HashMap, ) { let block = &dfg[block_id]; @@ -70,12 +75,22 @@ fn analyze_last_uses( instructions_that_can_be_made_mutable.remove(&existing); } } - Instruction::ArraySet { array, .. } => { + Instruction::ArraySet { array, value, .. } => { let array = dfg.resolve(*array); if let Some(existing) = array_to_last_use.insert(array, *instruction_id) { instructions_that_can_be_made_mutable.remove(&existing); } + if is_brillig_func { + let value = dfg.resolve(*value); + + if let Some(existing) = inner_nested_arrays.get(&value) { + instructions_that_can_be_made_mutable.remove(existing); + } + let result = dfg.instruction_results(*instruction_id)[0]; + inner_nested_arrays.insert(result, *instruction_id); + } + // If the array we are setting does not come from a load we can safely mark it mutable. // If the array comes from a load we may potentially being mutating an array at a reference // that is loaded from by other values. @@ -169,29 +184,31 @@ mod tests { // from and cloned in a loop. If the array is inadvertently marked mutable, and is cloned in a previous iteration // of the loop, its clone will also be altered. // - // acir(inline) fn main f0 { + // brillig fn main f0 { // b0(): - // v2 = allocate - // store [Field 0, Field 0, Field 0, Field 0, Field 0] at v2 // v3 = allocate - // store [Field 0, Field 0, Field 0, Field 0, Field 0] at v3 + // store [[Field 0, Field 0, Field 0, Field 0, Field 0], [Field 0, Field 0, Field 0, Field 0, Field 0]] at v3 + // v4 = allocate + // store [[Field 0, Field 0, Field 0, Field 0, Field 0], [Field 0, Field 0, Field 0, Field 0, Field 0]] at v4 // jmp b1(u32 0) - // b1(v5: u32): - // v7 = lt v5, u32 5 - // jmpif v7 then: b3, else: b2 + // b1(v6: u32): + // v8 = lt v6, u32 5 + // jmpif v8 then: b3, else: b2 // b3(): - // v8 = eq v5, u32 5 - // jmpif v8 then: b4, else: b5 + // v9 = eq v6, u32 5 + // jmpif v9 then: b4, else: b5 // b4(): - // v9 = load v2 - // store v9 at v3 + // v10 = load v3 + // store v10 at v4 // jmp b5() // b5(): - // v10 = load v2 - // v12 = array_set v10, index v5, value Field 20 - // store v12 at v2 - // v14 = add v5, u32 1 - // jmp b1(v14) + // v11 = load v3 + // v13 = array_get v11, index Field 0 + // v14 = array_set v13, index v6, value Field 20 + // v15 = array_set v11, index v6, value v14 + // store v15 at v3 + // v17 = add v6, u32 1 + // jmp b1(v17) // b2(): // return // } @@ -203,13 +220,16 @@ mod tests { let zero = builder.field_constant(0u128); let array_constant = builder.array_constant(vector![zero, zero, zero, zero, zero], array_type.clone()); + let nested_array_type = Type::Array(Arc::new(vec![array_type.clone()]), 2); + let nested_array_constant = builder + .array_constant(vector![array_constant, array_constant], nested_array_type.clone()); - let v2 = builder.insert_allocate(array_type.clone()); + let v3 = builder.insert_allocate(array_type.clone()); - builder.insert_store(v2, array_constant); + builder.insert_store(v3, nested_array_constant); - let v3 = builder.insert_allocate(array_type.clone()); - builder.insert_store(v3, array_constant); + let v4 = builder.insert_allocate(array_type.clone()); + builder.insert_store(v4, nested_array_constant); let b1 = builder.insert_block(); let zero_u32 = builder.numeric_constant(0u128, Type::unsigned(32)); @@ -219,35 +239,38 @@ mod tests { builder.switch_to_block(b1); let v5 = builder.add_block_parameter(b1, Type::unsigned(32)); let five = builder.numeric_constant(5u128, Type::unsigned(32)); - let v7 = builder.insert_binary(v5, BinaryOp::Lt, five); + let v8 = builder.insert_binary(v5, BinaryOp::Lt, five); let b2 = builder.insert_block(); let b3 = builder.insert_block(); let b4 = builder.insert_block(); let b5 = builder.insert_block(); - builder.terminate_with_jmpif(v7, b3, b2); + builder.terminate_with_jmpif(v8, b3, b2); // Loop body // b3 is the if statement conditional builder.switch_to_block(b3); let two = builder.numeric_constant(5u128, Type::unsigned(32)); - let v8 = builder.insert_binary(v5, BinaryOp::Eq, two); - builder.terminate_with_jmpif(v8, b4, b5); + let v9 = builder.insert_binary(v5, BinaryOp::Eq, two); + builder.terminate_with_jmpif(v9, b4, b5); // b4 is the rest of the loop after the if statement builder.switch_to_block(b4); - let v9 = builder.insert_load(v2, array_type.clone()); - builder.insert_store(v3, v9); + let v10 = builder.insert_load(v3, nested_array_type.clone()); + builder.insert_store(v4, v10); builder.terminate_with_jmp(b5, vec![]); builder.switch_to_block(b5); - let v10 = builder.insert_load(v2, array_type.clone()); + let v11 = builder.insert_load(v3, nested_array_type.clone()); let twenty = builder.field_constant(20u128); - let v12 = builder.insert_array_set(v10, v5, twenty); - builder.insert_store(v2, v12); + let v13 = builder.insert_array_get(v11, zero, array_type.clone()); + let v14 = builder.insert_array_set(v13, v5, twenty); + let v15 = builder.insert_array_set(v11, v5, v14); + + builder.insert_store(v3, v15); let one = builder.numeric_constant(1u128, Type::unsigned(32)); - let v14 = builder.insert_binary(v5, BinaryOp::Add, one); - builder.terminate_with_jmp(b1, vec![v14]); + let v17 = builder.insert_binary(v5, BinaryOp::Add, one); + builder.terminate_with_jmp(b1, vec![v17]); builder.switch_to_block(b2); builder.terminate_with_return(vec![]); @@ -265,7 +288,7 @@ mod tests { .filter(|instruction| matches!(&main.dfg[**instruction], Instruction::ArraySet { .. })) .collect::>(); - assert_eq!(array_set_instructions.len(), 1); + assert_eq!(array_set_instructions.len(), 2); if let Instruction::ArraySet { mutable, .. } = &main.dfg[*array_set_instructions[0]] { // The single array set should not be marked mutable assert!(!mutable);