Skip to content

Commit

Permalink
fix(ssa): Do not mark an array from a parameter mutable (#6355)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #6349 

## Summary\*

We check that we do not mark arrays mutable which comes from load
instructions as that array may be used by multiple values. This is
overridden by checking whether we are in the return block, in which case
we do mark the array as mutable. This however breaks when we no longer
inline everything. I switched the set of arrays from loads to a map of
arrays from loads to whether they come from a block parameter.

## Additional Context



## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
vezenovm authored Oct 25, 2024
1 parent 7c98b36 commit bcd8976
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions compiler/noirc_evaluator/src/ssa/opt/array_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ struct Context<'f> {
is_brillig_runtime: bool,
array_to_last_use: HashMap<ValueId, InstructionId>,
instructions_that_can_be_made_mutable: HashSet<InstructionId>,
arrays_from_load: HashSet<ValueId>,
// Mapping of an array that comes from a load and whether the address
// it was loaded from is a reference parameter.
arrays_from_load: HashMap<ValueId, bool>,
inner_nested_arrays: HashMap<ValueId, InstructionId>,
}

Expand All @@ -64,7 +66,7 @@ impl<'f> Context<'f> {
is_brillig_runtime,
array_to_last_use: HashMap::default(),
instructions_that_can_be_made_mutable: HashSet::default(),
arrays_from_load: HashSet::default(),
arrays_from_load: HashMap::default(),
inner_nested_arrays: HashMap::default(),
}
}
Expand Down Expand Up @@ -113,9 +115,13 @@ impl<'f> Context<'f> {
array_in_terminator = true;
}
});
if (!self.arrays_from_load.contains(&array) || is_return_block)
&& !array_in_terminator
{
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 {
self.instructions_that_can_be_made_mutable.insert(*instruction_id);
}
}
Expand All @@ -133,10 +139,12 @@ impl<'f> Context<'f> {
}
}
}
Instruction::Load { .. } => {
Instruction::Load { address } => {
let result = self.dfg.instruction_results(*instruction_id)[0];
if matches!(self.dfg.type_of_value(result), Array { .. } | Slice { .. }) {
self.arrays_from_load.insert(result);
let is_reference_param =
self.dfg.block_parameters(block_id).contains(address);
self.arrays_from_load.insert(result, is_reference_param);
}
}
_ => (),
Expand Down

0 comments on commit bcd8976

Please sign in to comment.