feat(performance): brillig array set optimization

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

@@ -804,8 +804,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 offseted = if dfg.is_constant(*index) {
-                    // For constant indices it must be the case that they have been offseted during SSA
+                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 {
@@ -816,7 +816,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                     array_variable,
                     index_variable,
                     destination_variable,
-                    offseted,
+                    has_offset,
                 );
             }
             Instruction::ArraySet { array, index, value, mutable, offset } => {
@@ -832,8 +832,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                     dfg,
                 );
 
-                let offseted = if dfg.is_constant(*index) {
-                    // For constant indices it must be the case that they have been offseted during SSA
+                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 {
@@ -846,7 +846,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
                     index_register,
                     value_variable,
                     *mutable,
-                    offseted,
+                    has_offset,
                 );
             }
             Instruction::RangeCheck { value, max_bit_size, assert_message } => {
@@ -1118,14 +1118,12 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
         array_variable: BrilligVariable,
         index_variable: SingleAddrVariable,
         destination_variable: BrilligVariable,
-        offseted: bool,
+        has_offset: bool,
     ) {
-        let (items_pointer, deallocate) = if offseted {
-            (array_variable.extract_register(), false)
+        let items_pointer = if has_offset {
+            array_variable.extract_register()
         } else {
-            let items_pointer =
-                self.brillig_context.codegen_make_array_or_vector_items_pointer(array_variable);
-            (items_pointer, true)
+            self.brillig_context.codegen_make_array_or_vector_items_pointer(array_variable)
         };
 
         self.brillig_context.codegen_load_with_offset(
@@ -1134,7 +1132,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
             destination_variable.extract_register(),
         );
 
-        if deallocate {
+        if !has_offset {
             self.brillig_context.deallocate_register(items_pointer);
         }
     }
@@ -1151,7 +1149,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
         index_register: SingleAddrVariable,
         value_variable: BrilligVariable,
         mutable: bool,
-        offseted: bool,
+        has_offset: bool,
     ) {
         assert!(index_register.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE);
         match (source_variable, destination_variable) {
@@ -1178,13 +1176,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
         let destination_for_store = if mutable { source_variable } else { destination_variable };
 
         // Then set the value in the newly created array
-        let (items_pointer, deallocate) = if offseted {
-            (destination_for_store.extract_register(), false)
+        let items_pointer = if has_offset {
+            destination_for_store.extract_register()
         } else {
-            let items_pointer = self
-                .brillig_context
-                .codegen_make_array_or_vector_items_pointer(destination_for_store);
-            (items_pointer, true)
+            self.brillig_context.codegen_make_array_or_vector_items_pointer(destination_for_store)
         };
 
         self.brillig_context.codegen_store_with_offset(
@@ -1201,7 +1196,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
             );
         }
 
-        if deallocate {
+        if !has_offset {
             self.brillig_context.deallocate_register(items_pointer);
         }
     }

compiler/noirc_evaluator/src/ssa.rs

@@ -210,7 +210,7 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
         // We can safely place the pass before DIE as that pass only removes instructions.
         // We also need DIE's tracking of used globals in case the array get transformations
         // end up using an existing constant from the globals space.
-        SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get Optimizations"),
+        SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
         SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
         // A function can be potentially unreachable post-DIE if all calls to that function were removed.
         SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"),
@@ -248,7 +248,7 @@ pub fn minimal_passes() -> Vec<SsaPass<'static>> {
         // We need a DIE pass to populate `used_globals`, otherwise it will panic later.
         SsaPass::new(Ssa::dead_instruction_elimination, "Dead Instruction Elimination"),
         // We need to add an offset to constant array indices in Brillig.
-        SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get Optimizations"),
+        SsaPass::new(Ssa::brillig_array_get_and_set, "Brillig Array Get and Set Optimizations"),
     ]
 }
 

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

@@ -280,13 +280,13 @@
     EnableSideEffectsIf { condition: ValueId },
 
     /// Retrieve a value from an array at the given index
-    /// `offset` determines whether the index has been offseted by some offset.
+    /// `offset` determines whether the index has been shifted by some offset.
     ArrayGet { array: ValueId, index: ValueId, offset: ArrayOffset },
 
     /// Creates a new array with the new value at the given index. All other elements are identical
     /// to those in the given array. This will not modify the original array unless `mutable` is
     /// set. This flag is off by default and only enabled when optimizations determine it is safe.
-    /// `offset` determines whether the index has been offseted by some offset.
+    /// `offset` determines whether the index has been shifted by some offset.
     ArraySet { array: ValueId, index: ValueId, value: ValueId, mutable: bool, offset: ArrayOffset },
 
     /// An instruction to increment the reference count of a value.
@@ -652,9 +652,9 @@
     }
 }
 
-/// Determines whether an ArrayGet or ArraySet index has been offseted by a given value.
+/// Determines whether an ArrayGet or ArraySet index has been shifted by a given value.
 /// Offsets are set during `crate::ssa::opt::brillig_array_gets` for brillig arrays
 /// and vectors with constant indicces.
 #[derive(Debug, PartialEq, Eq, Hash, Copy, Clone, Serialize, Deserialize)]
 pub enum ArrayOffset {
     None,

compiler/noirc_evaluator/src/ssa/parser/tests.rs

@@ -342,7 +342,7 @@ fn test_array_get() {
 }
 
 #[test]
-fn test_array_get_offseted_by_1() {
+fn test_array_get_with_index_minus_1() {
     let src: &'static str = "
         acir(inline) fn main f0 {
           b0(v0: [Field; 3]):
@@ -354,7 +354,7 @@ fn test_array_get_offseted_by_1() {
 }
 
 #[test]
-fn test_array_get_offseted_by_3() {
+fn test_array_get_with_index_minus_3() {
     let src: &'static str = "
         acir(inline) fn main f0 {
           b0(v0: [Field; 3]):
@@ -390,7 +390,7 @@ fn test_mutable_array_set() {
 }
 
 #[test]
-fn test_array_set_offseted_by_1() {
+fn test_array_set_with_index_minus_1() {
     let src = "
         acir(inline) fn main f0 {
           b0(v0: [Field; 3]):
@@ -402,7 +402,7 @@ fn test_array_set_offseted_by_1() {
 }
 
 #[test]
-fn test_array_set_offseted_by_3() {
+fn test_array_set_with_index_minus_3() {
     let src = "
         acir(inline) fn main f0 {
           b0(v0: [Field; 3]):