chore(ssa): Remove values from array type (#771)

* intialise abi arrays with store instructions

* code review - style

* code review

* code review

Author: guipublic

crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs

@@ -55,13 +55,9 @@ impl AcirMem {
     // Loads the associated `InternalVar` for the element
     // in the `array` at the given `offset`.
     //
-    // First we check if the address of the array element
-    // is in the memory_map. If not, then we check the `array`
+    // We check if the address of the array element
+    // is in the memory_map.
     //
-    // We do not check the `MemArray` initially because the
-    // `MemoryMap` holds the most updated InternalVar
-    // associated to the array element.
-    // TODO: specify what could change between the two?
     //
     // Returns `None` if `offset` is out of bounds.
     pub(crate) fn load_array_element_constant_index(
@@ -76,19 +72,10 @@ impl AcirMem {
         }
 
         // Check the memory_map to see if the element is there
-        if let Some(internal_var) = self.array_map_mut(array.id).get(&offset) {
-            return Some(internal_var.clone());
-        };
-
-        let array_element = array.values[offset as usize].clone();
-
-        // Compiler sanity check
-        //
-        // Since the only time we take the array values
-        // from the array is when it has been defined in the
-        // ABI. We know that it must have been initialized with a `Witness`
-        array_element.cached_witness().expect("ICE: since the value is not in the memory_map it must have came from the ABI, so it is a Witness");
-
-        Some(array_element)
+        let array_element = self
+            .array_map_mut(array.id)
+            .get(&offset)
+            .expect("ICE: Could not find value at index {offset}");
+        Some(array_element.clone())
     }
 }

crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs

@@ -45,9 +45,6 @@ impl InternalVar {
     pub(crate) fn set_id(&mut self, id: NodeId) {
         self.id = Some(id)
     }
-    pub(crate) fn cached_witness(&self) -> &Option<Witness> {
-        &self.cached_witness
-    }
 
     pub fn to_expression(&self) -> Expression {
         if let Some(w) = self.cached_witness {

crates/noirc_evaluator/src/ssa/conditional.rs

@@ -438,17 +438,6 @@ impl DecisionTree {
         Ok(())
     }
 
-    //assigns the arrays to the block where they are seen for the first time
-    fn new_array(ctx: &SsaContext, array_id: super::mem::ArrayId, stack: &mut StackFrame) {
-        if let std::collections::hash_map::Entry::Vacant(e) = stack.created_arrays.entry(array_id) {
-            if !ctx.mem[array_id].values.is_empty() {
-                e.insert(ctx.first_block);
-            } else {
-                e.insert(stack.block);
-            }
-        }
-    }
-
     fn short_circuit(
         ctx: &mut SsaContext,
         stack: &mut StackFrame,
@@ -511,17 +500,17 @@ impl DecisionTree {
         match &ins1.operation {
             Operation::Call { returned_arrays, .. } => {
                 for a in returned_arrays {
-                    DecisionTree::new_array(ctx, a.0, stack);
+                    stack.new_array(a.0);
                 }
             }
             Operation::Store { array_id, index, .. } => {
                 if *index != NodeId::dummy() {
-                    DecisionTree::new_array(ctx, *array_id, stack);
+                    stack.new_array(*array_id);
                 }
             }
             _ => {
                 if let ObjectType::Pointer(a) = ins1.res_type {
-                    DecisionTree::new_array(ctx, a, stack);
+                    stack.new_array(a);
                 }
             }
         }
@@ -718,7 +707,7 @@ impl DecisionTree {
                             val_false: ctx.one(),
                         };
                         if ctx.is_zero(*expr) {
-                            stack.clear();
+                            stack.stack.clear();
                         }
                         let cond = ctx.add_instruction(Instruction::new(
                             operation,

crates/noirc_evaluator/src/ssa/context.rs

@@ -907,12 +907,25 @@ impl SsaContext {
     }
 
     fn init_array(&mut self, array_id: ArrayId, stack_frame: &mut StackFrame) {
-        let len = self.mem[array_id].len;
+        let len = self.mem[array_id].len as usize;
         let e_type = self.mem[array_id].element_type;
-        for i in 0..len {
+        let values = vec![self.zero_with_type(e_type); len];
+        self.init_array_from_values(array_id, values, stack_frame);
+    }
+
+    pub fn init_array_from_values(
+        &mut self,
+        array_id: ArrayId,
+        values: Vec<NodeId>,
+        stack_frame: &mut StackFrame,
+    ) {
+        let len = self.mem[array_id].len as usize;
+        let e_type = self.mem[array_id].element_type;
+        assert_eq!(len, values.len());
+        for (i, v) in values.iter().enumerate() {
             let index =
                 self.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
-            let op_a = Operation::Store { array_id, index, value: self.zero_with_type(e_type) };
+            let op_a = Operation::Store { array_id, index, value: *v };
             self.new_instruction_inline(op_a, e_type, stack_frame);
         }
     }

crates/noirc_evaluator/src/ssa/inline.rs

@@ -114,13 +114,6 @@ impl StackFrame {
         }
     }
 
-    pub fn clear(&mut self) {
-        self.stack.clear();
-        self.array_map.clear();
-        self.created_arrays.clear();
-        self.lca_cache.clear();
-    }
-
     pub fn push(&mut self, ins_id: NodeId) {
         self.stack.push(ins_id);
     }
@@ -179,6 +172,13 @@ impl StackFrame {
             true
         }
     }
+
+    //assigns the arrays to the block where they are seen for the first time
+    pub fn new_array(&mut self, array_id: ArrayId) {
+        if let std::collections::hash_map::Entry::Vacant(e) = self.created_arrays.entry(array_id) {
+            e.insert(self.block);
+        }
+    }
 }
 
 //inline a function call

crates/noirc_evaluator/src/ssa/mem.rs

@@ -1,5 +1,4 @@
 use crate::ssa::{
-    acir_gen::InternalVar,
     context::SsaContext,
     node,
     node::{Node, NodeId},
@@ -30,7 +29,6 @@ impl ArrayId {
 pub struct MemArray {
     pub id: ArrayId,
     pub element_type: node::ObjectType, //type of elements
-    pub values: Vec<InternalVar>,
     pub name: String,
     pub def: Definition,
     pub len: u32,     //number of elements
@@ -51,7 +49,6 @@ impl MemArray {
             id,
             element_type: of,
             name: name.to_string(),
-            values: Vec::new(),
             def: definition,
             len,
             adr: 0,

crates/noirc_evaluator/src/ssa/ssa_gen.rs

@@ -96,8 +96,21 @@ impl IrGenerator {
     ) -> NodeId {
         let element_type = self.get_object_type_from_abi(el_type);
         let (v_id, array_idx) = self.new_array(name, element_type, len as u32, ident_def);
-        self.context.mem[array_idx].values = vecmap(witness, |w| w.into());
-        self.context.get_current_block_mut().update_variable(v_id, v_id);
+        let values = vecmap(witness.iter().enumerate(), |(i, w)| {
+            let mut var = Variable::new(
+                element_type,
+                format!("{name}_{i}"),
+                None,
+                self.context.current_block,
+            );
+            var.witness = Some(*w);
+            self.context.add_variable(var, None)
+        });
+        let mut stack_frame = crate::ssa::inline::StackFrame::new(self.context.current_block);
+        self.context.init_array_from_values(array_idx, values, &mut stack_frame);
+        let block = self.context.get_current_block_mut();
+        block.instructions.extend_from_slice(&stack_frame.stack);
+        block.update_variable(v_id, v_id);
         v_id
     }