Merge 3fb0a0e into 29bd125

noir-lang · Oct 7, 2024 · 9007d60 · 9007d60
2 parents 29bd125 + 3fb0a0e
commit 9007d60
Show file tree

Hide file tree

Showing 5 changed files with 221 additions and 38 deletions.
diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs
@@ -1,5 +1,7 @@
+use std::sync::Arc;
+
 use acvm::{acir::AcirField, FieldElement};
-use fxhash::{FxHashMap as HashMap, FxHashSet};
+use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
 
 use crate::ssa::ir::{
     basic_block::BasicBlockId,
@@ -152,57 +154,32 @@ impl<'a> ValueMerger<'a> {
         then_value: ValueId,
         else_value: ValueId,
     ) -> ValueId {
-        let mut merged = im::Vector::new();
-
-        let (element_types, len) = match &typ {
+        let (_element_types, len) = match &typ {
             Type::Array(elements, len) => (elements, *len),
             _ => panic!("Expected array type"),
         };
 
-        let actual_length = len * element_types.len();
-
         if let Some(result) = self.try_merge_only_changed_indices(
             then_condition,
             else_condition,
             then_value,
             else_value,
-            actual_length,
+            len,
         ) {
             return result;
         }
 
-        for i in 0..len {
-            for (element_index, element_type) in element_types.iter().enumerate() {
-                let index = ((i * element_types.len() + element_index) as u128).into();
-                let index = self.dfg.make_constant(index, Type::field());
-
-                let typevars = Some(vec![element_type.clone()]);
-
-                let mut get_element = |array, typevars| {
-                    let get = Instruction::ArrayGet { array, index };
-                    self.dfg
-                        .insert_instruction_and_results(
-                            get,
-                            self.block,
-                            typevars,
-                            self.call_stack.clone(),
-                        )
-                        .first()
-                };
-
-                let then_element = get_element(then_value, typevars.clone());
-                let else_element = get_element(else_value, typevars);
+        let outer_type = Type::Array(Arc::new(vec![typ.clone()]), 2);
+        let new_array = self.dfg.make_array(vec![else_value, then_value].into(), outer_type);
 
-                merged.push_back(self.merge_values(
-                    then_condition,
-                    else_condition,
-                    then_element,
-                    else_element,
-                ));
-            }
-        }
+        let index =
+            self.insert_instruction(Instruction::Cast(then_condition, Type::length_type())).first();
 
-        self.dfg.make_array(merged, typ)
+        let get = Instruction::ArrayGet { array: new_array, index };
+        let typevars = Some(vec![typ]);
+        self.dfg
+            .insert_instruction_and_results(get, self.block, typevars, self.call_stack.clone())
+            .first()
     }
 
     fn merge_slice_values(
@@ -366,7 +343,7 @@ impl<'a> ValueMerger<'a> {
             current_else = self.find_previous_array_set(current_else, &mut seen_else);
         }
 
-        let changed_indices: FxHashSet<_> = seen_then
+        let changed_indices: HashSet<_> = seen_then
             .into_iter()
             .map(|(_, index, typ, condition)| (index, typ, condition))
             .chain(seen_else.into_iter().map(|(_, index, typ, condition)| (index, typ, condition)))

diff --git a/test_programs/execution_success/regression_5027/Nargo.toml b/test_programs/execution_success/regression_5027/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "regression_5027"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.33.0"
+
+[dependencies]
diff --git a/test_programs/execution_success/regression_5027/Prover.toml b/test_programs/execution_success/regression_5027/Prover.toml
@@ -0,0 +1,9 @@
+fields = [
+    0, 0,
+]
+enables = [
+    true, false,
+]
+indices = [
+    0, 0
+]
diff --git a/test_programs/execution_success/regression_5027/Prover.toml.old b/test_programs/execution_success/regression_5027/Prover.toml.old
@@ -0,0 +1,168 @@
+fields = [
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+]
+enables = [
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+    true, false, true, false, true, false, true, false, true, false,
+]
+indices = [
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+]
diff --git a/test_programs/execution_success/regression_5027/src/main.nr b/test_programs/execution_success/regression_5027/src/main.nr
@@ -0,0 +1,22 @@
+// ACIR opcodes should now increase linearly per each increase in SIZE:
+// | SIZE | ACIR Opcodes | Difference
+// | 2    | 45           |
+// | 3    | 67           | +22
+// | 4    | 89           | +22
+// | 5    | 111          | +22
+// ...
+// | 500  | 11001        |
+global SIZE = 2;
+
+fn main(mut fields: [Field; SIZE], enables: [bool; SIZE], indices: [u64; SIZE]) -> pub [Field; SIZE] {
+    for i in 0..SIZE {
+        if enables[i] {
+            // Expect two optimizations:
+            // 1: Shortcut array merge to only merge 1 changed index each time
+            // 2: Each array_set created by the above should be mutable
+            fields[indices[i]] = i as Field;
+        }
+    }
+
+    fields
+}