chore: Re-open #3258 (#3410)

Co-authored-by: Tom French <[email protected]>
Co-authored-by: Tom French <[email protected]>
Co-authored-by: jfecher <[email protected]>

Author: 3 people

compiler/noirc_evaluator/src/ssa.rs

@@ -12,7 +12,10 @@ use std::{
     ops::Range,
 };
 
-use crate::errors::{RuntimeError, SsaReport};
+use crate::{
+    brillig::Brillig,
+    errors::{RuntimeError, SsaReport},
+};
 use acvm::acir::{
     circuit::{Circuit, PublicInputs},
     native_types::Witness,
@@ -42,7 +45,8 @@ pub(crate) fn optimize_into_acir(
     print_brillig_trace: bool,
 ) -> Result<GeneratedAcir, RuntimeError> {
     let abi_distinctness = program.return_distinctness;
-    let ssa = SsaBuilder::new(program, print_ssa_passes)?
+
+    let ssa_builder = SsaBuilder::new(program, print_ssa_passes)?
         .run_pass(Ssa::defunctionalize, "After Defunctionalization:")
         .run_pass(Ssa::inline_functions, "After Inlining:")
         // Run mem2reg with the CFG separated into blocks
@@ -59,10 +63,17 @@ pub(crate) fn optimize_into_acir(
         // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores
         .run_pass(Ssa::mem2reg, "After Mem2Reg:")
         .run_pass(Ssa::fold_constants, "After Constant Folding:")
-        .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
+        .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:");
+
+    let brillig = ssa_builder.to_brillig(print_brillig_trace);
+
+    // Split off any passes the are not necessary for Brillig generation but are necessary for ACIR generation.
+    // We only need to fill out nested slices as we need to have a known length when dealing with memory operations
+    // in ACIR gen while this is not necessary in the Brillig IR.
+    let ssa = ssa_builder
+        .run_pass(Ssa::fill_internal_slices, "After Fill Internal Slice Dummy Data:")
         .finish();
 
-    let brillig = ssa.to_brillig(print_brillig_trace);
     let last_array_uses = ssa.find_last_array_uses();
     ssa.into_acir(brillig, abi_distinctness, &last_array_uses)
 }
@@ -156,6 +167,10 @@ impl SsaBuilder {
         Ok(self.print(msg))
     }
 
+    fn to_brillig(&self, print_brillig_trace: bool) -> Brillig {
+        self.ssa.to_brillig(print_brillig_trace)
+    }
+
     fn print(self, msg: &str) -> Self {
         if self.print_ssa_passes {
             println!("{msg}\n{}", self.ssa);

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -424,7 +424,9 @@ impl Context {
                             let rhs_var = read_from_index(rhs_block_id, i)?;
                             Ok((lhs_var, rhs_var))
                         }),
-                        _ => unreachable!("ICE: lhs and rhs should be of the same type"),
+                        _ => {
+                            unreachable!("ICE: lhs and rhs should be of the same type")
+                        }
                     }
                 }
 
@@ -502,7 +504,7 @@ impl Context {
                                     self.initialize_array(block_id, len, Some(output.clone()))?;
                                 }
                                 AcirValue::DynamicArray(_) => {
-                                    unreachable!("The output from an intrinsic call is expected to be a single value or an array but got {output:?}");
+                                    // Do nothing as a dynamic array returned from a slice intrinsic should already be initialized
                                 }
                                 AcirValue::Var(_, _) => {
                                     // Do nothing
@@ -1018,7 +1020,8 @@ impl Context {
             }
         }
 
-        let element_type_sizes = self.init_element_type_sizes_array(&array_typ, array_id, dfg)?;
+        let element_type_sizes =
+            self.init_element_type_sizes_array(&array_typ, array_id, None, dfg)?;
         let result_value = AcirValue::DynamicArray(AcirDynamicArray {
             block_id: result_block_id,
             len: array_len,
@@ -1105,6 +1108,7 @@ impl Context {
         &mut self,
         array_typ: &Type,
         array_id: ValueId,
+        array_acir_value: Option<AcirValue>,
         dfg: &DataFlowGraph,
     ) -> Result<BlockId, RuntimeError> {
         let element_type_sizes = self.internal_block_id(&array_id);
@@ -1132,7 +1136,11 @@ impl Context {
                     Value::Instruction { .. } | Value::Param { .. } => {
                         // An instruction representing the slice means it has been processed previously during ACIR gen.
                         // Use the previously defined result of an array operation to fetch the internal type information.
-                        let array_acir_value = self.convert_value(array_id, dfg);
+                        let array_acir_value = if let Some(array_acir_value) = array_acir_value {
+                            array_acir_value
+                        } else {
+                            self.convert_value(array_id, dfg)
+                        };
                         match array_acir_value {
                             AcirValue::DynamicArray(AcirDynamicArray {
                                 element_type_sizes: inner_elem_type_sizes,
@@ -1274,7 +1282,8 @@ impl Context {
         var_index: AcirVar,
         dfg: &DataFlowGraph,
     ) -> Result<AcirVar, RuntimeError> {
-        let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, dfg)?;
+        let element_type_sizes =
+            self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?;
 
         let predicate_index =
             self.acir_context.mul_var(var_index, self.current_side_effects_enabled_var)?;
@@ -1728,26 +1737,50 @@ impl Context {
             }
             Intrinsic::SlicePushBack => {
                 let slice_length = self.convert_value(arguments[0], dfg).into_var()?;
+                let (array_id, array_typ, _) =
+                    self.check_array_is_initialized(arguments[1], dfg)?;
                 let slice = self.convert_value(arguments[1], dfg);
-                // TODO(#2461): make sure that we have handled nested struct inputs
-                let element = self.convert_value(arguments[2], dfg);
 
+                // TODO(#3364): make sure that we have handled nested struct inputs
+                let element = self.convert_value(arguments[2], dfg);
                 let one = self.acir_context.add_constant(FieldElement::one());
                 let new_slice_length = self.acir_context.add_var(slice_length, one)?;
 
+                // We attach the element no matter what in case len == capacity, as otherwise we
+                // may get an out of bounds error.
                 let mut new_slice = Vector::new();
-                self.slice_intrinsic_input(&mut new_slice, slice)?;
-                new_slice.push_back(element);
-
-                Ok(vec![
-                    AcirValue::Var(new_slice_length, AcirType::field()),
-                    AcirValue::Array(new_slice),
-                ])
+                self.slice_intrinsic_input(&mut new_slice, slice.clone())?;
+                new_slice.push_back(element.clone());
+
+                // TODO(#3364): This works for non-nested outputs
+                let len = Self::flattened_value_size(&slice);
+                let new_elem_size = Self::flattened_value_size(&element);
+                let new_slice_val = AcirValue::Array(new_slice);
+                let result_block_id = self.block_id(&result_ids[1]);
+                self.initialize_array(
+                    result_block_id,
+                    len + new_elem_size,
+                    Some(new_slice_val.clone()),
+                )?;
+                let mut var_index = slice_length;
+                self.array_set_value(element, result_block_id, &mut var_index)?;
+
+                let result = AcirValue::DynamicArray(AcirDynamicArray {
+                    block_id: result_block_id,
+                    len: len + new_elem_size,
+                    element_type_sizes: self.init_element_type_sizes_array(
+                        &array_typ,
+                        array_id,
+                        Some(new_slice_val),
+                        dfg,
+                    )?,
+                });
+                Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result])
             }
             Intrinsic::SlicePushFront => {
                 let slice_length = self.convert_value(arguments[0], dfg).into_var()?;
-                let slice = self.convert_value(arguments[1], dfg);
-                // TODO(#2461): make sure that we have handled nested struct inputs
+                let slice: AcirValue = self.convert_value(arguments[1], dfg);
+                // TODO(#3364): make sure that we have handled nested struct inputs
                 let element = self.convert_value(arguments[2], dfg);
 
                 let one = self.acir_context.add_constant(FieldElement::one());
@@ -1769,12 +1802,18 @@ impl Context {
                 let one = self.acir_context.add_constant(FieldElement::one());
                 let new_slice_length = self.acir_context.sub_var(slice_length, one)?;
 
+                let (_, _, block_id) = self.check_array_is_initialized(arguments[1], dfg)?;
+                let mut var_index = new_slice_length;
+                let elem = self.array_get_value(
+                    &dfg.type_of_value(result_ids[2]),
+                    block_id,
+                    &mut var_index,
+                    &[],
+                )?;
+
+                // TODO(#3364): make sure that we have handled nested struct inputs
                 let mut new_slice = Vector::new();
                 self.slice_intrinsic_input(&mut new_slice, slice)?;
-                // TODO(#2461): make sure that we have handled nested struct inputs
-                let elem = new_slice
-                    .pop_back()
-                    .expect("There are no elements in this slice to be removed");
 
                 Ok(vec![
                     AcirValue::Var(new_slice_length, AcirType::field()),
@@ -1791,7 +1830,7 @@ impl Context {
 
                 let mut new_slice = Vector::new();
                 self.slice_intrinsic_input(&mut new_slice, slice)?;
-                // TODO(#2461): make sure that we have handled nested struct inputs
+                // TODO(#3364): make sure that we have handled nested struct inputs
                 let elem = new_slice
                     .pop_front()
                     .expect("There are no elements in this slice to be removed");
@@ -1831,7 +1870,7 @@ impl Context {
                 // they are attempting to insert at too large of an index.
                 // This check prevents a panic inside of the im::Vector insert method.
                 if index <= new_slice.len() {
-                    // TODO(#2461): make sure that we have handled nested struct inputs
+                    // TODO(#3364): make sure that we have handled nested struct inputs
                     new_slice.insert(index, element);
                 }
 
@@ -1869,7 +1908,7 @@ impl Context {
                 // they are attempting to remove at too large of an index.
                 // This check prevents a panic inside of the im::Vector remove method.
                 let removed_elem = if index < new_slice.len() {
-                    // TODO(#2461): make sure that we have handled nested struct inputs
+                    // TODO(#3364): make sure that we have handled nested struct inputs
                     new_slice.remove(index)
                 } else {
                     // This is a dummy value which should never be used if the appropriate