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(perf): Follow array sets backwards in array set from get optimization #6208

Merged
merged 16 commits into from
Oct 4, 2024
Merged
Changes from 10 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: 68 additions & 6 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,25 +818,87 @@ fn try_optimize_array_get_from_previous_set(
SimplifyResult::None
}

/// If we have an array set whose value is from an array get on the same array at the same index,
/// we can simplify that array set to the array we were looking to perform an array set upon.
///
/// Simple case:
/// v3 = array_get v1, index v2
/// v5 = array_set v1, index v2, value v3
///
/// If we could not immediately simplify the array set from its value, we can try to follow
/// the array set backwards in the case we have constant indices:
///
/// v3 = array_get v1, index 1
/// v5 = array_set v1, index 2, value [Field 100, Field 101, Field 102]
/// v7 = array_set mut v5, index 1, value v3
///
/// We want to optimize `v7` to `v5`. We see that `v3` comes from an array get to `v1`. We follow `v5` backwards and see an array set
/// to `v1` and see that the previous array set occurs to a different constant index.
///
/// 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.
/// - Otherwise, we check the array value of the array set. We will refer to this array as array'.
/// In the case above, array' is `v1` from `v5 = array set ...`
/// - Check the original array set value's array get against array'. If they are equal we can simplify.
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
/// - Continuing the example above, as we have `v3 = array_get v1, index 1`, `v1` is
/// what we want to check against array'. We now know we can simplify `v7` to `v5` as it is unchanged.
/// - If they are not equal, recur
fn try_optimize_array_set_from_previous_get(
dfg: &DataFlowGraph,
array_id: ValueId,
mut array_id: ValueId,
target_index: ValueId,
target_value: ValueId,
) -> SimplifyResult {
match &dfg[target_value] {
let array_from_get = match &dfg[target_value] {
Value::Instruction { instruction, .. } => match &dfg[*instruction] {
Instruction::ArrayGet { array, index } => {
if *array == array_id && *index == target_index {
SimplifyResult::SimplifiedTo(array_id)
// If array and index match from the value, we can immediately simplify
return SimplifyResult::SimplifiedTo(array_id);
} else if *index == target_index {
*array
} else {
SimplifyResult::None
return SimplifyResult::None;
}
}
_ => SimplifyResult::None,
_ => return SimplifyResult::None,
},
_ => SimplifyResult::None,
_ => return SimplifyResult::None,
};

vezenovm marked this conversation as resolved.
Show resolved Hide resolved
let Some(target_index) = dfg.get_numeric_constant(target_index) else {
return SimplifyResult::None;
};

let original_array_id = array_id;
// 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, .. } => {
if let Some(index) = dfg.get_numeric_constant(*index) {
if index == target_index {
return SimplifyResult::None;
}

if *array == array_from_get {
return SimplifyResult::SimplifiedTo(original_array_id);
}

array_id = *array; // recur
} else {
return SimplifyResult::None;
}
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
}
_ => return SimplifyResult::None,
},
_ => return SimplifyResult::None,
}
}

SimplifyResult::None
}

pub(crate) type ErrorType = HirType;
Expand Down
Loading