chore(ACIRgen): always compute array offset

compiler/noirc_evaluator/src/acir/arrays.rs

@@ -217,19 +217,14 @@ impl Context<'_> {
         }
 
         let array_typ = dfg.type_of_value(array);
-        let offset = self.compute_offset(instruction, dfg, &array_typ);
-        let (new_index, new_value) = self.convert_array_operation_inputs(
-            array,
-            dfg,
-            index,
-            store_value,
-            offset.unwrap_or_default(),
-        )?;
+        let offset = self.compute_offset(instruction, dfg, &array_typ, store_value);
+        let (new_index, new_value) =
+            self.convert_array_operation_inputs(array, dfg, index, store_value, offset)?;
 
         if let Some(new_value) = new_value {
             self.array_set(instruction, new_index, new_value, dfg, mutable)?;
         } else {
-            self.array_get(instruction, array, new_index, dfg, offset.is_none())?;
+            self.array_get(instruction, array, new_index, dfg)?;
         }
 
         Ok(())
@@ -360,30 +355,34 @@ impl Context<'_> {
     }
 
     /// Get an offset such that the type of the array at the offset is the same as the type at the 'index'
-    /// If we find one, we will use it when computing the index under the enable_side_effect predicate
-    /// If not, array_get(..) will use a fallback costing one multiplication in the worst case.
-    /// cf. <https://github.com/noir-lang/noir/pull/4971>
-    /// For simplicity we compute the offset only for simple arrays
+    /// The offset is computed based on the type of the stored value, if any (if it's an ArraySet instruction),
+    /// or otherwise the type of the result (if it's an ArrayGet instruction).
     fn compute_offset(
         &mut self,
         instruction: InstructionId,
         dfg: &DataFlowGraph,
         array_typ: &Type,
-    ) -> Option<usize> {
-        let is_simple_array = dfg.instruction_results(instruction).len() == 1
-            && (array_has_constant_element_size(array_typ) == Some(1));
-        if is_simple_array {
-            let result_type = dfg.type_of_value(dfg.instruction_results(instruction)[0]);
-            match array_typ {
-                Type::Array(item_type, _) | Type::Slice(item_type) => item_type
-                    .iter()
-                    .enumerate()
-                    .find_map(|(index, typ)| (result_type == *typ).then_some(index)),
-                _ => None,
-            }
+        store_value: Option<ValueId>,
+    ) -> usize {
+        let value_at_index = if let Some(store_value) = store_value {
+            store_value
         } else {
-            None
+            let [result] = dfg.instruction_result(instruction);
+            result
+        };
+        let value_type = dfg.type_of_value(value_at_index);
+        match array_typ {
+            Type::Array(item_type, _) | Type::Slice(item_type) => {
+                for (index, typ) in item_type.iter().enumerate() {
+                    if &value_type == typ {
+                        return index;
+                    }
+                }
+            }
+            _ => (),
         }
+
+        unreachable!("ICE: should always be able to compute offset for array operation");
     }
 
     /// We need to properly setup the inputs for array operations in ACIR.
@@ -552,23 +551,19 @@ impl Context<'_> {
     }
 
     /// Generates a read opcode for the array
-    /// `index_side_effect == false` means that we ensured `var_index` will have a type matching the value in the array
     fn array_get(
         &mut self,
         instruction: InstructionId,
         array: ValueId,
         var_index: AcirVar,
         dfg: &DataFlowGraph,
-        index_side_effect: bool,
     ) -> Result<(), RuntimeError> {
         let block_id = self.ensure_array_is_initialized(array, dfg)?;
         let [result] = dfg.instruction_result(instruction);
         let res_typ = dfg.type_of_value(result);
 
         let value = self.load_array_value(array, block_id, var_index, &res_typ, dfg)?;
 
-        let value = self.apply_index_side_effects(array, value, index_side_effect, dfg)?;
-
         self.define_result(dfg, instruction, value);
         Ok(())
     }
@@ -598,37 +593,6 @@ impl Context<'_> {
         }
     }
 
-    /// Applies predication logic on the result in case the read under a false predicate
-    /// returns a value with a larger type that may later trigger an overflow.
-    /// Ensures values read under false predicate are zeroed out if types don’t align.
-    fn apply_index_side_effects(
-        &mut self,
-        array: ValueId,
-        mut value: AcirValue,
-        mut index_side_effect: bool,
-        dfg: &DataFlowGraph,
-    ) -> Result<AcirValue, RuntimeError> {
-        if let AcirValue::Var(value_var, typ) = &value {
-            let array_typ = dfg.type_of_value(array);
-            if let (Type::Numeric(numeric_type), AcirType::NumericType(num)) =
-                (array_typ.first(), typ)
-            {
-                if numeric_type.bit_size() <= num.bit_size() {
-                    // first element is compatible
-                    index_side_effect = false;
-                }
-            }
-
-            if index_side_effect {
-                value = AcirValue::Var(
-                    self.acir_context.mul_var(*value_var, self.current_side_effects_enabled_var)?,
-                    typ.clone(),
-                );
-            }
-        }
-        Ok(value)
-    }
-
     pub(super) fn array_get_value(
         &mut self,
         ssa_type: &Type,

compiler/noirc_evaluator/src/ssa/ir/types.rs

@@ -288,14 +288,6 @@ impl Type {
         }
     }
 
-    pub(crate) fn first(&self) -> Type {
-        match self {
-            Type::Numeric(_) | Type::Function => self.clone(),
-            Type::Reference(typ) => typ.first(),
-            Type::Slice(element_types) | Type::Array(element_types, _) => element_types[0].first(),
-        }
-    }
-
     /// True if this is a reference type or if it is a composite type which contains a reference.
     pub(crate) fn contains_reference(&self) -> bool {
         match self {