chore(ssa): Remove offset from ArrayGet and ArraySet

compiler/noirc_evaluator/src/acir/arrays.rs

@@ -201,8 +201,8 @@ impl Context<'_> {
 
         // Pass the instruction between array methods rather than the internal fields themselves
         let (array, index, store_value) = match dfg[instruction] {
-            Instruction::ArrayGet { array, index, offset: _ } => (array, index, None),
-            Instruction::ArraySet { array, index, value, mutable, offset: _ } => {
+            Instruction::ArrayGet { array, index } => (array, index, None),
+            Instruction::ArraySet { array, index, value, mutable } => {
                 mutable_array_set = mutable;
                 (array, index, Some(value))
             }

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -8,7 +8,7 @@ use crate::brillig::brillig_ir::registers::RegisterAllocator;
 use crate::brillig::brillig_ir::{
     BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, BrilligBinaryOp, BrilligContext, ReservedRegisters,
 };
-use crate::ssa::ir::instruction::{ArrayOffset, ConstrainError, Hint};
+use crate::ssa::ir::instruction::{ConstrainError, Hint};
 use crate::ssa::ir::{
     basic_block::BasicBlockId,
     dfg::DataFlowGraph,
@@ -846,7 +846,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                 let source_variable = self.convert_ssa_single_addr_value(*value, dfg);
                 self.convert_cast(destination_variable, source_variable);
             }
-            Instruction::ArrayGet { array, index, offset } => {
+            Instruction::ArrayGet { array, index } => {
                 let result_ids = dfg.instruction_results(instruction_id);
                 let destination_variable = self.variables.define_variable(
                     self.function_context,
@@ -858,13 +858,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                 let array_variable = self.convert_ssa_value(*array, dfg);
                 let index_variable = self.convert_ssa_single_addr_value(*index, dfg);
 
-                let has_offset = if dfg.is_constant(*index) {
-                    // For constant indices it must be the case that they have been offset during SSA
-                    assert!(*offset != ArrayOffset::None);
-                    true
-                } else {
-                    false
-                };
+                // Constants are assumed to have been offset just before Brillig gen.
+                let has_offset = dfg.is_constant(*index);
 
                 self.convert_ssa_array_get(
                     array_variable,
@@ -873,7 +868,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                     has_offset,
                 );
             }
-            Instruction::ArraySet { array, index, value, mutable, offset } => {
+            Instruction::ArraySet { array, index, value, mutable } => {
                 let source_variable = self.convert_ssa_value(*array, dfg);
                 let index_register = self.convert_ssa_single_addr_value(*index, dfg);
                 let value_variable = self.convert_ssa_value(*value, dfg);
@@ -886,13 +881,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                     dfg,
                 );
 
-                let has_offset = if dfg.is_constant(*index) {
-                    // For constant indices it must be the case that they have been offset during SSA
-                    assert!(*offset != ArrayOffset::None);
-                    true
-                } else {
-                    false
-                };
+                // Constants are assumed to have been offset just before Brillig gen.
+                let has_offset = dfg.is_constant(*index);
 
                 self.convert_ssa_array_set(
                     source_variable,

compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs

@@ -522,7 +522,7 @@ impl DependencyContext {
                     // For array get operations, we check the Brillig calls for
                     // results involving the array in question, to properly
                     // populate the array element tainted sets
-                    Instruction::ArrayGet { array, index, offset: _ } => {
+                    Instruction::ArrayGet { array, index } => {
                         self.process_array_get(*array, *index, &results, function);
                         // Record all the used arguments as parents of the results
                         self.update_children(&arguments, &results);

compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs

@@ -2,7 +2,6 @@ use std::{collections::BTreeMap, sync::Arc};
 
 use crate::ssa::ir::{
     function::RuntimeType,
-    instruction::ArrayOffset,
     types::{NumericType, Type},
     value::{ValueId, ValueMapping},
 };
@@ -167,9 +166,7 @@ impl FunctionBuilder {
                         if let Type::Array(_, 0) = subitem_typ {
                             continue;
                         }
-                        let offset = ArrayOffset::None;
-                        let element =
-                            self.insert_array_get(value, index_var, offset, subitem_typ.clone());
+                        let element = self.insert_array_get(value, index_var, subitem_typ.clone());
                         index += match subitem_typ {
                             Type::Array(_, _) | Type::Slice(_) => subitem_typ.element_size(),
                             Type::Numeric(_) => 1,

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

@@ -23,7 +23,7 @@ use super::{
         basic_block::BasicBlock,
         dfg::{GlobalsGraph, InsertInstructionResult},
         function::RuntimeType,
-        instruction::{ArrayOffset, ConstrainError, InstructionId, Intrinsic},
+        instruction::{ConstrainError, InstructionId, Intrinsic},
         types::NumericType,
     },
     opt::pure::FunctionPurities,
@@ -353,12 +353,10 @@ impl FunctionBuilder {
         &mut self,
         array: ValueId,
         index: ValueId,
-        offset: ArrayOffset,
         element_type: Type,
     ) -> ValueId {
         let element_type = Some(vec![element_type]);
-        self.insert_instruction(Instruction::ArrayGet { array, index, offset }, element_type)
-            .first()
+        self.insert_instruction(Instruction::ArrayGet { array, index }, element_type).first()
     }
 
     /// Insert an instruction to create a new array with the given index replaced with a new value
@@ -368,9 +366,8 @@ impl FunctionBuilder {
         index: ValueId,
         value: ValueId,
         mutable: bool,
-        offset: ArrayOffset,
     ) -> ValueId {
-        let instruction = Instruction::ArraySet { array, index, value, mutable, offset };
+        let instruction = Instruction::ArraySet { array, index, value, mutable };
         self.insert_instruction(instruction, None).first()
     }
 

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

@@ -5,9 +5,7 @@ use super::{
     ir::{
         dfg::DataFlowGraph,
         function::{Function, FunctionId, RuntimeType},
-        instruction::{
-            ArrayOffset, Binary, BinaryOp, ConstrainError, Instruction, TerminatorInstruction,
-        },
+        instruction::{Binary, BinaryOp, ConstrainError, Instruction, TerminatorInstruction},
         types::Type,
         value::ValueId,
     },
@@ -585,19 +583,17 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
                 self.side_effects_enabled = self.lookup_bool(*condition, "enable_side_effects")?;
                 Ok(())
             }
-            Instruction::ArrayGet { array, index, offset } => {
-                self.interpret_array_get(*array, *index, *offset, results[0], side_effects_enabled)
+            Instruction::ArrayGet { array, index } => {
+                self.interpret_array_get(*array, *index, results[0], side_effects_enabled)
             }
-            Instruction::ArraySet { array, index, value, mutable, offset } => self
-                .interpret_array_set(
-                    *array,
-                    *index,
-                    *value,
-                    *mutable,
-                    *offset,
-                    results[0],
-                    side_effects_enabled,
-                ),
+            Instruction::ArraySet { array, index, value, mutable } => self.interpret_array_set(
+                *array,
+                *index,
+                *value,
+                *mutable,
+                results[0],
+                side_effects_enabled,
+            ),
             Instruction::IncrementRc { value } => self.interpret_inc_rc(*value),
             Instruction::DecrementRc { value } => self.interpret_dec_rc(*value),
             Instruction::IfElse { then_condition, then_value, else_condition, else_value } => self
@@ -892,10 +888,10 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
         &mut self,
         array: ValueId,
         index: ValueId,
-        offset: ArrayOffset,
         result: ValueId,
         side_effects_enabled: bool,
     ) -> IResult<()> {
+        let offset = self.dfg().array_offset(array, index);
         let array = self.lookup_array_or_slice(array, "array get")?;
         let index = self.lookup_u32(index, "array get index")?;
         let mut index = index - offset.to_u32();
@@ -959,10 +955,10 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
         index: ValueId,
         value: ValueId,
         mutable: bool,
-        offset: ArrayOffset,
         result: ValueId,
         side_effects_enabled: bool,
     ) -> IResult<()> {
+        let offset = self.dfg().array_offset(array, index);
         let array = self.lookup_array_or_slice(array, "array set")?;
 
         let result_array = if side_effects_enabled {

compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs

@@ -490,9 +490,9 @@ fn shr_signed() {
         "
         acir(inline) fn main f0 {
           b0():
-            v0 = shr i16 65520, i16 2      
-            v1 = shr i16 65533, i16 1      
-            v2 = shr i16 65528, i16 3 
+            v0 = shr i16 65520, i16 2
+            v1 = shr i16 65533, i16 1
+            v2 = shr i16 65528, i16 3
             return v0, v1, v2
         }
     ",
@@ -829,10 +829,10 @@ fn array_get() {
 fn array_get_with_offset() {
     let value = expect_value(
         r#"
-        acir(inline) fn main f0 {
+        brillig(inline) fn main f0 {
           b0():
             v0 = make_array [Field 1, Field 2] : [Field; 2]
-            v1 = array_get v0, index u32 4 minus 3 -> Field
+            v1 = array_get v0, index u32 2 minus 1 -> Field
             return v1
         }
     "#,
@@ -949,10 +949,11 @@ fn array_set_disabled_by_enable_side_effects() {
 fn array_set_with_offset() {
     let values = expect_values(
         "
-        acir(inline) fn main f0 {
+        brillig(inline) fn main f0 {
           b0():
             v0 = make_array [Field 1, Field 2] : [Field; 2]
-            v1 = array_set v0, index u32 4 minus 3, value Field 5
+            inc_rc v0
+            v1 = array_set v0, index u32 2 minus 1, value Field 5
             return v0, v1
         }
     ",
@@ -961,18 +962,15 @@ fn array_set_with_offset() {
     let v0 = values[0].as_array_or_slice().unwrap();
     let v1 = values[1].as_array_or_slice().unwrap();
 
-    // acir function, so all rcs are 1
-    assert_eq!(*v0.rc.borrow(), 1);
+    assert_eq!(*v0.rc.borrow(), 2);
     assert_eq!(*v1.rc.borrow(), 1);
 
     let one = from_constant(1u32.into(), NumericType::NativeField);
     let two = from_constant(2u32.into(), NumericType::NativeField);
     let five = from_constant(5u32.into(), NumericType::NativeField);
 
-    // v0 was not mutated
-    assert_eq!(*v0.elements.borrow(), vec![one.clone(), two.clone()]);
-    // v1 was mutated
-    assert_eq!(*v1.elements.borrow(), vec![one, five]);
+    assert_eq!(*v0.elements.borrow(), vec![one.clone(), two.clone()], "v0 should not be mutated");
+    assert_eq!(*v1.elements.borrow(), vec![one, five], "v1 should be mutated");
 }
 
 #[test]

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

@@ -2,6 +2,7 @@ use std::{borrow::Cow, sync::Arc};
 
 use crate::ssa::{
     function_builder::data_bus::DataBus,
+    ir::instruction::ArrayOffset,
     opt::pure::{FunctionPurities, Purity},
 };
 
@@ -107,6 +108,9 @@ pub(crate) struct DataFlowGraph {
 
     #[serde(skip)]
     pub(crate) function_purities: Arc<FunctionPurities>,
+
+    /// Indicate whether the Brillig array index offset optimizations have been performed.
+    pub(crate) brillig_arrays_offset: bool,
 }
 
 /// The GlobalsGraph contains the actual global data.
@@ -766,6 +770,21 @@ impl DataFlowGraph {
     pub(crate) fn purity_of(&self, function: FunctionId) -> Option<Purity> {
         self.function_purities.get(&function).copied()
     }
+
+    /// Determine the appropriate [ArrayOffset] to use for indexing an array or slice.
+    pub(crate) fn array_offset(&self, array: ValueId, index: ValueId) -> ArrayOffset {
+        if !self.runtime.is_brillig()
+            || !self.brillig_arrays_offset
+            || self.get_numeric_constant(index).is_none()
+        {
+            return ArrayOffset::None;
+        }
+        match self.type_of_value(array) {
+            Type::Array(_, _) => ArrayOffset::Array,
+            Type::Slice(_) => ArrayOffset::Slice,
+            _ => ArrayOffset::None,
+        }
+    }
 }
 
 impl std::ops::Index<InstructionId> for DataFlowGraph {

compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs

@@ -2,7 +2,7 @@ use crate::ssa::{
     ir::{
         basic_block::BasicBlockId,
         instruction::{
-            ArrayOffset, Binary, BinaryOp, ConstrainError, Instruction,
+            Binary, BinaryOp, ConstrainError, Instruction,
             binary::{truncate, truncate_field},
         },
         types::{NumericType, Type},
@@ -110,7 +110,7 @@ pub(crate) fn simplify(
             }
         }
         Instruction::ConstrainNotEqual(..) => None,
-        Instruction::ArrayGet { array, index, offset } => {
+        Instruction::ArrayGet { array, index } => {
             if let Some(index) = dfg.get_numeric_constant(*index) {
                 return try_optimize_array_get_from_previous_set(dfg, *array, index);
             }
@@ -121,7 +121,7 @@ pub(crate) fn simplify(
             {
                 // If the array is of length 1 then we know the only value which can be potentially read out of it.
                 // We can then simply assert that the index is equal to zero and return the array's contained value.
-                optimize_length_one_array_read(dfg, block, call_stack, *array, *index, *offset)
+                optimize_length_one_array_read(dfg, block, call_stack, *array, *index)
             } else {
                 None
             }
@@ -342,7 +342,6 @@ fn optimize_length_one_array_read(
     call_stack: CallStackId,
     array: ValueId,
     index: ValueId,
-    offset: ArrayOffset,
 ) -> SimplifyResult {
     let zero = dfg.make_constant(FieldElement::zero(), NumericType::length_type());
     let index_constraint = Instruction::Constrain(
@@ -354,11 +353,7 @@ fn optimize_length_one_array_read(
 
     let result = try_optimize_array_get_from_previous_set(dfg, array, FieldElement::zero());
     if let SimplifyResult::None = result {
-        SimplifyResult::SimplifiedToInstruction(Instruction::ArrayGet {
-            array,
-            index: zero,
-            offset,
-        })
+        SimplifyResult::SimplifiedToInstruction(Instruction::ArrayGet { array, index: zero })
     } else {
         result
     }
@@ -456,8 +451,8 @@ fn try_optimize_array_set_from_previous_get(
     target_value: ValueId,
 ) -> SimplifyResult {
     let array_from_get = match dfg.get_local_or_global_instruction(target_value) {
-        Some(Instruction::ArrayGet { array, index, offset }) => {
-            if *offset == ArrayOffset::None && *array == array_id && *index == target_index {
+        Some(Instruction::ArrayGet { array, index }) => {
+            if *array == array_id && *index == target_index {
                 // If array and index match from the value, we can immediately simplify
                 return SimplifyResult::SimplifiedTo(array_id);
             } else if *index == target_index {

compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/call.rs

@@ -11,7 +11,7 @@ use crate::ssa::{
     ir::{
         basic_block::BasicBlockId,
         dfg::DataFlowGraph,
-        instruction::{ArrayOffset, Binary, BinaryOp, Endian, Hint, Instruction, Intrinsic},
+        instruction::{Binary, BinaryOp, Endian, Hint, Instruction, Intrinsic},
         types::{NumericType, Type},
         value::{Value, ValueId},
     },
@@ -511,7 +511,6 @@ fn simplify_slice_push_back(
         index: arguments[0],
         value: arguments[2],
         mutable: false,
-        offset: ArrayOffset::None,
     };
 
     let set_last_slice_value = dfg
@@ -565,11 +564,8 @@ fn simplify_slice_pop_back(
     // Iterating through element types in reverse here since we're popping from the end
     for element_type in element_types.iter().rev() {
         flattened_len = decrement_slice_length(flattened_len, dfg, block, call_stack);
-        let get_last_elem_instr = Instruction::ArrayGet {
-            array: arguments[1],
-            index: flattened_len,
-            offset: ArrayOffset::None,
-        };
+        let get_last_elem_instr =
+            Instruction::ArrayGet { array: arguments[1], index: flattened_len };
 
         let element_type = Some(vec![element_type.clone()]);
         let get_last_elem = dfg

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

@@ -125,6 +125,7 @@ impl Function {
         new_function.set_runtime(another.runtime());
         new_function.set_globals(another.dfg.globals.clone());
         new_function.dfg.set_function_purities(another.dfg.function_purities.clone());
+        new_function.dfg.brillig_arrays_offset = another.dfg.brillig_arrays_offset;
         new_function
     }