From 59ab111589c105e3aaedf56dbdfdc4151a579e76 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 19 Sep 2025 16:08:26 +0100 Subject: [PATCH 1/5] Separate out remove_enable_side_effect pre_check --- .../src/ssa/opt/remove_enable_side_effects.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index 6983580a464..3a899359b35 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -35,6 +35,9 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn remove_enable_side_effects(mut self) -> Ssa { for function in self.functions.values_mut() { + #[cfg(debug_assertions)] + remove_enable_side_effects_pre_check(function); + function.remove_enable_side_effects(); } self @@ -48,10 +51,6 @@ impl Function { return; } - // Check the precondition that this optimization runs when there's only one block - let block = self.entry_block(); - assert_eq!(self.dfg[block].successors().count(), 0); - let one = self.dfg.make_constant(FieldElement::one(), NumericType::bool()); let mut active_condition = one; let mut last_side_effects_enabled_instruction = None; @@ -87,6 +86,7 @@ impl Function { if condition_is_one { last_side_effects_enabled_instruction = None; + context.insert_current_instruction(); } else { last_side_effects_enabled_instruction = Some(context.instruction_id); context.remove_current_instruction(); @@ -107,6 +107,16 @@ impl Function { } } +/// Check that the CFG has been flattened. +#[cfg(debug_assertions)] +fn remove_enable_side_effects_pre_check(function: &Function) { + if !function.runtime().is_acir() { + return; + } + let block = function.entry_block(); + assert_eq!(function.dfg[block].successors().count(), 0); +} + #[cfg(test)] mod test { use crate::{ @@ -258,7 +268,7 @@ mod test { v9 = array_set v7, index v0, value Field 4 // this instruction should be removed - enable_side_effects v1 + enable_side_effects v1 v13, v14 = call slice_push_back(u32 3, v9, Field 5) -> (u32, [Field]) return From f6eaa1d3bab199da3dfc0e36330f8d3375fcc2e6 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 19 Sep 2025 17:00:00 +0100 Subject: [PATCH 2/5] Validate array operand --- .../noirc_evaluator/src/ssa/validation/mod.rs | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/validation/mod.rs b/compiler/noirc_evaluator/src/ssa/validation/mod.rs index a149269166c..eb24ec806e5 100644 --- a/compiler/noirc_evaluator/src/ssa/validation/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/validation/mod.rs @@ -172,11 +172,16 @@ impl<'f> Validator<'f> { panic!("Cannot use `{operator}` with field elements"); }; } - Instruction::ArrayGet { index, .. } | Instruction::ArraySet { index, .. } => { + Instruction::ArrayGet { array, index, .. } + | Instruction::ArraySet { array, index, .. } => { let index_type = dfg.type_of_value(*index); if !matches!(index_type, Type::Numeric(NumericType::Unsigned { bit_size: 32 })) { panic!("ArrayGet/ArraySet index must be u32"); } + let array_type = dfg.type_of_value(*array); + if !array_type.contains_an_array() { + panic!("ArrayGet/ArraySet must operate on an array; got {array_type}"); + } } Instruction::Call { func, arguments } => { self.type_check_call(instruction, func, arguments); @@ -1606,7 +1611,7 @@ mod tests { return } - + "; let _ = Ssa::from_str(src).unwrap(); } @@ -1617,7 +1622,7 @@ mod tests { let src = " acir(inline) fn main f0 { b0(): - v3 = make_array [Field 1, Field 2, Field 3] : [Field] + v3 = make_array [Field 1, Field 2, Field 3] : [Field] v4 = call f1(v3) -> u32 return v4 } @@ -1658,4 +1663,34 @@ mod tests { "; let _ = Ssa::from_str(src).unwrap(); } + + #[test] + #[should_panic(expected = "ArrayGet/ArraySet index must be u32")] + fn array_get_wrong_index_type() { + let src = format!( + r#" + acir(inline) predicate_pure fn main f0 {{ + b0(v0: [u8; 3], v1: u64): + v2 = array_get v0, index v1 -> u32 + return v2 + }} + "# + ); + let _ = Ssa::from_str(&src).unwrap(); + } + + #[test] + #[should_panic(expected = "ArrayGet/ArraySet must operate on an array")] + fn array_get_wrong_array_type() { + let src = format!( + r#" + acir(inline) predicate_pure fn main f0 {{ + b0(v0: u32, v1: u32): + v2 = array_get v0, index v1 -> u32 + return v2 + }} + "# + ); + let _ = Ssa::from_str(&src).unwrap(); + } } From 08038c7629d7c269d35b2ddc073aba51a7c1c40f Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 19 Sep 2025 17:01:49 +0100 Subject: [PATCH 3/5] Fix array inputs in unit tests --- .../src/ssa/opt/remove_enable_side_effects.rs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index 3a899359b35..d436fe617aa 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -262,10 +262,10 @@ mod test { fn remove_enable_side_effects_for_slice_push_back() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): + b0(v0: [u32; 3], v1: u1, v2: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 + v9 = array_set v7, index v2, value Field 4 // this instruction should be removed enable_side_effects v1 @@ -278,11 +278,11 @@ mod test { let ssa = ssa.remove_enable_side_effects(); assert_ssa_snapshot!(ssa, @r" acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): - v3 = array_get v0, index u32 0 -> u32 - v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 - v13, v14 = call slice_push_back(u32 3, v9, Field 5) -> (u32, [Field]) + b0(v0: [u32; 3], v1: u1, v2: u32): + v4 = array_get v0, index u32 0 -> u32 + v8 = make_array [Field 1, Field 2, Field 3] : [Field] + v10 = array_set v8, index v2, value Field 4 + v14, v15 = call slice_push_back(u32 3, v10, Field 5) -> (u32, [Field]) return } "); @@ -292,7 +292,7 @@ mod test { fn remove_enable_side_effects_for_slice_push_front() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): + b0(v0: [u32; 3], v1: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] v9 = array_set v7, index v0, value Field 4 @@ -322,10 +322,10 @@ mod test { fn keep_enable_side_effects_for_slice_pop_back() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): + b0(v0: [u32; 3], v1: u1, v2: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 + v9 = array_set v7, index v2, value Field 4 enable_side_effects v1 v13, v14, v15 = call slice_pop_back(u32 3, v9) -> (u32, [Field], Field) return @@ -338,10 +338,10 @@ mod test { fn keep_enable_side_effects_for_slice_pop_front() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): + b0(v0: [u32; 3], v1: u1, v2: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 + v9 = array_set v7, index v2, value Field 4 enable_side_effects v1 v13, v14, v15 = call slice_pop_front(u32 3, v9) -> (Field, u32, [Field]) return @@ -354,7 +354,7 @@ mod test { fn keep_enable_side_effects_for_slice_insert() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): + b0(v0: [u32; 3], v1: u1): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] v9 = array_set v7, index v0, value Field 4 @@ -370,7 +370,7 @@ mod test { fn keep_enable_side_effects_for_slice_remove() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): + b0(v0: [u32; 3], v1: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] v9 = array_set v7, index v0, value Field 4 From 50a997bd439c7f6c38cca748680356a2f8a5863e Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 19 Sep 2025 17:07:38 +0100 Subject: [PATCH 4/5] No need for format --- .../noirc_evaluator/src/ssa/validation/mod.rs | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/validation/mod.rs b/compiler/noirc_evaluator/src/ssa/validation/mod.rs index eb24ec806e5..fb66fa91e86 100644 --- a/compiler/noirc_evaluator/src/ssa/validation/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/validation/mod.rs @@ -1667,30 +1667,24 @@ mod tests { #[test] #[should_panic(expected = "ArrayGet/ArraySet index must be u32")] fn array_get_wrong_index_type() { - let src = format!( - r#" - acir(inline) predicate_pure fn main f0 {{ + let src = " + acir(inline) predicate_pure fn main f0 { b0(v0: [u8; 3], v1: u64): v2 = array_get v0, index v1 -> u32 return v2 - }} - "# - ); - let _ = Ssa::from_str(&src).unwrap(); + }"; + let _ = Ssa::from_str(src).unwrap(); } #[test] #[should_panic(expected = "ArrayGet/ArraySet must operate on an array")] fn array_get_wrong_array_type() { - let src = format!( - r#" - acir(inline) predicate_pure fn main f0 {{ + let src = " + acir(inline) predicate_pure fn main f0 { b0(v0: u32, v1: u32): v2 = array_get v0, index v1 -> u32 return v2 - }} - "# - ); - let _ = Ssa::from_str(&src).unwrap(); + }"; + let _ = Ssa::from_str(src).unwrap(); } } From 3175621a8a5c191ecacb442236d1e1e5f0afe618 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 19 Sep 2025 17:59:31 +0100 Subject: [PATCH 5/5] Fix more unit tests --- .../src/ssa/opt/remove_enable_side_effects.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index d436fe617aa..3617fab0dce 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -292,10 +292,10 @@ mod test { fn remove_enable_side_effects_for_slice_push_front() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: [u32; 3], v1: u32): + b0(v0: [u32; 3], v1: u1, v2: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 + v9 = array_set v7, index v2, value Field 4 // this instruction should be removed enable_side_effects v1 @@ -308,11 +308,11 @@ mod test { let ssa = ssa.remove_enable_side_effects(); assert_ssa_snapshot!(ssa, @r" acir(inline) predicate_pure fn main f0 { - b0(v0: u32, v1: u1): - v3 = array_get v0, index u32 0 -> u32 - v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 - v13, v14 = call slice_push_front(u32 3, v9, Field 5) -> (u32, [Field]) + b0(v0: [u32; 3], v1: u1, v2: u32): + v4 = array_get v0, index u32 0 -> u32 + v8 = make_array [Field 1, Field 2, Field 3] : [Field] + v10 = array_set v8, index v2, value Field 4 + v14, v15 = call slice_push_front(u32 3, v10, Field 5) -> (u32, [Field]) return } "); @@ -354,10 +354,10 @@ mod test { fn keep_enable_side_effects_for_slice_insert() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: [u32; 3], v1: u1): + b0(v0: [u32; 3], v1: u1, v2: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 + v9 = array_set v7, index v2, value Field 4 enable_side_effects v1 v13, v14 = call slice_insert(u32 3, v9, u32 1, Field 5) -> (u32, [Field]) return @@ -370,10 +370,10 @@ mod test { fn keep_enable_side_effects_for_slice_remove() { let src = " acir(inline) predicate_pure fn main f0 { - b0(v0: [u32; 3], v1: u32): + b0(v0: [u32; 3], v1: u1, v2: u32): v3 = array_get v0, index u32 0 -> u32 v7 = make_array [Field 1, Field 2, Field 3] : [Field] - v9 = array_set v7, index v0, value Field 4 + v9 = array_set v7, index v2, value Field 4 enable_side_effects v1 v13, v14, v15 = call slice_remove(u32 3, v9, u32 1) -> (u32, [Field], Field) return