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.

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

feat(optimization): Follow past array_sets when optimizing array_gets #5772

Merged
merged 7 commits into from
Aug 20, 2024
Merged
Changes from 5 commits
Commits
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
74 changes: 65 additions & 9 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,16 +608,11 @@ impl Instruction {
}
}
Instruction::ArrayGet { array, index } => {
let array = dfg.get_array_constant(*array);
let index = dfg.get_numeric_constant(*index);
if let (Some((array, _)), Some(index)) = (array, index) {
let index =
index.try_to_u32().expect("Expected array index to fit in u32") as usize;
if index < array.len() {
return SimplifiedTo(array[index]);
}
if let Some(index) = dfg.get_numeric_constant(*index) {
try_optimize_array_get_from_previous_set(dfg, *array, index)
} else {
None
}
None
}
Instruction::ArraySet { array, index, value, .. } => {
let array = dfg.get_array_constant(*array);
Expand Down Expand Up @@ -744,6 +739,67 @@ impl Instruction {
}
}

/// Given a chain of operations like:
/// v1 = array_set [10, 11, 12], index 1, value: 5
/// v2 = array_set v1, index 2, value: 6
/// v3 = array_set v2, index 2, value: 7
/// v4 = array_get v3, index 1
///
/// We want to optimize `v4` to `10`. To do this we need to follow the array value
/// through several array sets. For each array set:
/// - If the index is non-constant we fail the optimization since any index may be changed
/// - If the index is constant and is our target index, we conservatively fail the optimization
/// in case the array_set is disabled from a previous `enable_side_effects_if` and the array get
/// was not.
/// - Otherwise, we check the array value of the array set.
/// - If the array value is constant, we use that array.
/// - If the array value is from a previous array-set, we recur.
fn try_optimize_array_get_from_previous_set(
dfg: &mut DataFlowGraph,
jfecher marked this conversation as resolved.
Show resolved Hide resolved
mut array_id: Id<Value>,
target_index: FieldElement,
) -> SimplifyResult {
let mut elements = None;

// Arbitrary number of maximum tries just to prevent this optimization from taking too long.
let max_tries = 5;
for _ in 0..max_tries {
match &dfg[array_id] {
Value::Instruction { instruction, .. } => {
match &dfg[*instruction] {
Instruction::ArraySet { array, index, value, .. } => {
if let Some(constant) = dfg.get_numeric_constant(*index) {
if constant == target_index {
return SimplifyResult::SimplifiedTo(*value);
}

array_id = *array; // recur
} else {
return SimplifyResult::None;
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
}
}
_ => return SimplifyResult::None,
}
}
Value::Array { array, typ: _ } => {
elements = Some(array.clone());
break;
}
_ => return SimplifyResult::None,
}
}

if let Some(array) = elements {
if let Some(index) = target_index.try_to_u64() {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let index = index as usize;
if index < array.len() {
return SimplifyResult::SimplifiedTo(array[index]);
}
}
}
SimplifyResult::None
}

pub(crate) type ErrorType = HirType;

pub(crate) fn error_selector_from_type(typ: &ErrorType) -> ErrorSelector {
Expand Down
Loading