diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index 363378d26ce..15fced2415f 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs @@ -1,4 +1,3 @@ -use acvm::{FieldElement, acir::AcirField}; use noirc_errors::call_stack::CallStackId; use rustc_hash::FxHashMap as HashMap; @@ -214,6 +213,9 @@ impl<'a> ValueMerger<'a> { let len = then_len.max(else_len); + let flattened_then_length = then_len * element_types.len() as u32; + let flattened_else_length = else_len * element_types.len() as u32; + for i in 0..len { for (element_index, element_type) in element_types.iter().enumerate() { let index_u32 = i * element_types.len() as u32 + element_index as u32; @@ -223,10 +225,8 @@ impl<'a> ValueMerger<'a> { let typevars = Some(vec![element_type.clone()]); let mut get_element = |array, typevars, len| { - // The smaller slice is filled with placeholder data. Codegen for slice accesses must - // include checks against the dynamic slice length so that this placeholder data is not incorrectly accessed. if len <= index_u32 { - self.make_slice_dummy_data(element_type) + panic!("get_element invoked with an out of bounds index"); } else { let offset = ArrayOffset::None; let get = Instruction::ArrayGet { array, index, offset }; @@ -240,11 +240,26 @@ impl<'a> ValueMerger<'a> { } }; - let len = then_len * element_types.len() as u32; - let then_element = get_element(then_value_id, typevars.clone(), len); + // If it's out of bounds for the "then" slice, a value in the "else" *must* exist. + // We can use that value directly as accessing it is always checked against the actual + // slice length. + if index_u32 >= flattened_then_length { + let else_element = get_element(else_value_id, typevars, flattened_else_length); + merged.push_back(else_element); + continue; + } + + // Same for if it's out of bounds for the "else" slice. + if index_u32 >= flattened_else_length { + let then_element = + get_element(then_value_id, typevars.clone(), flattened_then_length); + merged.push_back(then_element); + continue; + } - let len = else_len * element_types.len() as u32; - let else_element = get_element(else_value_id, typevars, len); + let then_element = + get_element(then_value_id, typevars.clone(), flattened_then_length); + let else_element = get_element(else_value_id, typevars, flattened_else_length); merged.push_back(self.merge_values( then_condition, @@ -261,41 +276,4 @@ impl<'a> ValueMerger<'a> { self.dfg.insert_instruction_and_results(instruction, self.block, None, call_stack); Ok(result.first()) } - - /// Construct a dummy value to be attached to the smaller of two slices being merged. - /// We need to make sure we follow the internal element type structure of the slice type - /// even for dummy data to ensure that we do not have errors later in the compiler, - /// such as with dynamic indexing of non-homogenous slices. - fn make_slice_dummy_data(&mut self, typ: &Type) -> ValueId { - match typ { - Type::Numeric(numeric_type) => { - let zero = FieldElement::zero(); - self.dfg.make_constant(zero, *numeric_type) - } - Type::Array(element_types, len) => { - let mut array = im::Vector::new(); - for _ in 0..*len { - for typ in element_types.iter() { - array.push_back(self.make_slice_dummy_data(typ)); - } - } - let instruction = Instruction::MakeArray { elements: array, typ: typ.clone() }; - let call_stack = self.call_stack; - self.dfg - .insert_instruction_and_results(instruction, self.block, None, call_stack) - .first() - } - Type::Slice(_) => { - // TODO(#3188): Need to update flattening to use true user facing length of slices - // to accurately construct dummy data - unreachable!("ICE: Cannot return a slice of slices from an if expression") - } - Type::Reference(_) => { - unreachable!("ICE: Merging references is unsupported") - } - Type::Function => { - unreachable!("ICE: Merging functions is unsupported") - } - } - } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index a306186ce35..1b6f7436e02 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -554,30 +554,27 @@ acir(inline) impure fn main f0 { v10 = not v0 v11 = cast v0 as u32 v13 = array_get v9, index u32 0 -> Field - v14 = cast v0 as Field - v15 = cast v10 as Field - v16 = mul v14, v13 - v17 = make_array [v16] : [Field] + v14 = make_array [v13] : [Field] enable_side_effects u1 1 - v20 = eq v11, u32 1 - v21 = not v20 - v22 = add v11, u32 1 - v23 = make_array [v16, v2] : [Field] - v24 = array_set v23, index v11, value v2 - v25 = array_get v24, index u32 0 -> Field - v26 = cast v21 as Field - v27 = cast v20 as Field - v28 = mul v26, v25 - v29 = mul v27, v16 - v30 = add v28, v29 - v31 = array_get v24, index u32 1 -> Field - v32 = cast v21 as Field - v33 = cast v20 as Field - v34 = mul v32, v31 - v35 = mul v33, v2 - v36 = add v34, v35 - v37 = make_array [v30, v36] : [Field] - constrain v30 == Field 1 + v17 = eq v11, u32 1 + v18 = not v17 + v19 = add v11, u32 1 + v20 = make_array [v13, v2] : [Field] + v21 = array_set v20, index v11, value v2 + v22 = array_get v21, index u32 0 -> Field + v23 = cast v18 as Field + v24 = cast v17 as Field + v25 = mul v23, v22 + v26 = mul v24, v13 + v27 = add v25, v26 + v28 = array_get v21, index u32 1 -> Field + v29 = cast v18 as Field + v30 = cast v17 as Field + v31 = mul v29, v28 + v32 = mul v30, v2 + v33 = add v31, v32 + v34 = make_array [v27, v33] : [Field] + constrain v27 == Field 1 return } ");