Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(optimization): Perform array gets as early as possible #5762

Closed
wants to merge 27 commits into from
Closed

feat(optimization): Perform array gets as early as possible #5762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9dc626d
Add pass to move array gets
jfecher Aug 19, 2024
913b629
Fix tests
jfecher Aug 19, 2024
d2a63da
Add test program
jfecher Aug 19, 2024
99d04db
Add comment
jfecher Aug 19, 2024
c94a858
Try increasing size of value merge optimization
jfecher Aug 19, 2024
8e96a55
Update compiler/noirc_evaluator/src/ssa/opt/move_array_gets.rs
jfecher Aug 19, 2024
5ead726
Update compiler/noirc_evaluator/src/ssa/opt/move_array_gets.rs
jfecher Aug 19, 2024
91d2312
Add comment, tweak constant
jfecher Aug 19, 2024
7856616
Merge branch 'jf/move-array-gets' of https://github.com/noir-lang/noi…
jfecher Aug 19, 2024
63a8ec0
Revert constant value
jfecher Aug 19, 2024
9268c93
Revert constant again
jfecher Aug 19, 2024
244ef0d
Try changing value merge optimization
jfecher Aug 20, 2024
41d3787
Lower constant; rename test; remove unused code
jfecher Aug 20, 2024
4005fe9
Reduce constant to see runtime difference
jfecher Aug 20, 2024
0cede20
Actually just don't array_get at all
jfecher Aug 20, 2024
459893e
Remove ssa file
jfecher Aug 21, 2024
107562e
Fix end condition
jfecher Aug 21, 2024
0553153
Use a less radical approach instead
jfecher Aug 21, 2024
aa8cd90
Fix warning
jfecher Aug 21, 2024
798ef0f
Merge branch 'master' into jf/move-array-gets
jfecher Aug 23, 2024
427e387
Try crazy optimization
jfecher Aug 23, 2024
f771698
Fmt
jfecher Aug 23, 2024
b5dfcc7
Use odd optimization only on fallback full array merge
jfecher Aug 23, 2024
d30d0b4
Merge branch 'jf/move-array-gets' of https://github.com/noir-lang/noi…
jfecher Aug 23, 2024
fd6ebef
Merge branch 'master' into jf/move-array-gets
TomAFrench Sep 4, 2024
8c517e9
.
TomAFrench Oct 1, 2024
3fb0a0e
Merge branch 'master' into jf/move-array-gets
TomAFrench Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 15 additions & 38 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)))
Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_success/regression_5027/Nargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_5027"
type = "bin"
authors = [""]
compiler_version = ">=0.33.0"

[dependencies]
9 changes: 9 additions & 0 deletions test_programs/execution_success/regression_5027/Prover.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fields = [
0, 0,
]
enables = [
true, false,
]
indices = [
0, 0
]
168 changes: 168 additions & 0 deletions test_programs/execution_success/regression_5027/Prover.toml.old
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
]
22 changes: 22 additions & 0 deletions test_programs/execution_success/regression_5027/src/main.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
}
Loading