Skip to content

Commit

Permalink
chore: pull out array_set pass changes (#9993)
Browse files Browse the repository at this point in the history
Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.
  • Loading branch information
TomAFrench authored Nov 18, 2024
1 parent 41743d0 commit db6fdc2
Showing 1 changed file with 27 additions and 24 deletions.
51 changes: 27 additions & 24 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/array_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,21 @@ impl Ssa {

impl Function {
pub(crate) fn array_set_optimization(&mut self) {
if matches!(self.runtime(), RuntimeType::Brillig(_)) {
// Brillig is supposed to use refcounting to decide whether to mutate an array;
// array mutation was only meant for ACIR. We could use it with Brillig as well,
// but then some of the optimizations that we can do in ACIR around shared
// references have to be skipped, which makes it more cumbersome.
return;
}

let reachable_blocks = self.reachable_blocks();

if !self.runtime().is_entry_point() {
assert_eq!(reachable_blocks.len(), 1, "Expected there to be 1 block remaining in Acir function for array_set optimization");
}

let mut context =
Context::new(&self.dfg, matches!(self.runtime(), RuntimeType::Brillig(_)));
let mut context = Context::new(&self.dfg);

for block in reachable_blocks.iter() {
context.analyze_last_uses(*block);
Expand All @@ -50,20 +57,18 @@ impl Function {

struct Context<'f> {
dfg: &'f DataFlowGraph,
is_brillig_runtime: bool,
array_to_last_use: HashMap<ValueId, InstructionId>,
instructions_that_can_be_made_mutable: HashSet<InstructionId>,
// Mapping of an array that comes from a load and whether the address
// it was loaded from is a reference parameter.
// it was loaded from is a reference parameter passed to the block.
arrays_from_load: HashMap<ValueId, bool>,
inner_nested_arrays: HashMap<ValueId, InstructionId>,
}

impl<'f> Context<'f> {
fn new(dfg: &'f DataFlowGraph, is_brillig_runtime: bool) -> Self {
fn new(dfg: &'f DataFlowGraph) -> Self {
Context {
dfg,
is_brillig_runtime,
array_to_last_use: HashMap::default(),
instructions_that_can_be_made_mutable: HashSet::default(),
arrays_from_load: HashMap::default(),
Expand All @@ -85,43 +90,41 @@ impl<'f> Context<'f> {
self.instructions_that_can_be_made_mutable.remove(&existing);
}
}
Instruction::ArraySet { array, value, .. } => {
Instruction::ArraySet { array, .. } => {
let array = self.dfg.resolve(*array);

if let Some(existing) = self.array_to_last_use.insert(array, *instruction_id) {
self.instructions_that_can_be_made_mutable.remove(&existing);
}
if self.is_brillig_runtime {
let value = self.dfg.resolve(*value);

if let Some(existing) = self.inner_nested_arrays.get(&value) {
self.instructions_that_can_be_made_mutable.remove(existing);
}
let result = self.dfg.instruction_results(*instruction_id)[0];
self.inner_nested_arrays.insert(result, *instruction_id);
}

// If the array we are setting does not come from a load we can safely mark it mutable.
// If the array comes from a load we may potentially being mutating an array at a reference
// that is loaded from by other values.
let terminator = self.dfg[block_id].unwrap_terminator();

// If we are in a return block we are not concerned about the array potentially being mutated again.
let is_return_block =
matches!(terminator, TerminatorInstruction::Return { .. });

// We also want to check that the array is not part of the terminator arguments, as this means it is used again.
let mut array_in_terminator = false;
let mut is_array_in_terminator = false;
terminator.for_each_value(|value| {
if value == array {
array_in_terminator = true;
// The terminator can contain original IDs, while the SSA has replaced the array value IDs; we need to resolve to compare.
if !is_array_in_terminator && self.dfg.resolve(value) == array {
is_array_in_terminator = true;
}
});
if let Some(is_from_param) = self.arrays_from_load.get(&array) {

let can_mutate = if let Some(is_from_param) = self.arrays_from_load.get(&array)
{
// If the array was loaded from a reference parameter, we cannot
// safely mark that array mutable as it may be shared by another value.
if !is_from_param && is_return_block {
self.instructions_that_can_be_made_mutable.insert(*instruction_id);
}
} else if !array_in_terminator {
!is_from_param && is_return_block
} else {
!is_array_in_terminator
};

if can_mutate {
self.instructions_that_can_be_made_mutable.insert(*instruction_id);
}
}
Expand Down

0 comments on commit db6fdc2

Please sign in to comment.