Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 7 additions & 6 deletions compiler/noirc_evaluator/src/ssa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,13 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass<'_>> {
Ssa::verify_no_dynamic_indices_to_references,
"Verifying no dynamic array indices to reference value elements",
),
SsaPass::new(Ssa::array_set_optimization, "Array Set Optimizations").and_then(|ssa| {
// Deferred sanity checks that don't modify the SSA, just panic if we have something unexpected
// that we don't know how to attribute to a concrete error with the Noir code.
ssa.dead_instruction_elimination_post_check(true);
ssa
}),
SsaPass::new(Ssa::mutable_array_set_optimization, "Mutable Array Set Optimizations")
.and_then(|ssa| {
// Deferred sanity checks that don't modify the SSA, just panic if we have something unexpected
// that we don't know how to attribute to a concrete error with the Noir code.
ssa.dead_instruction_elimination_post_check(true);
ssa
}),
]
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
//! Generally, these passes are also expected to minimize the final amount of instructions.

mod array_get;
mod array_set;
mod as_vector_length;
mod basic_conditional;
mod black_box_bypass;
Expand All @@ -28,6 +27,7 @@ mod inlining;
mod loop_invariant;
mod make_constrain_not_equal;
mod mem2reg;
mod mutable_array_set;
mod normalize_value_ids;
mod preprocess_fns;
pub(crate) mod pure;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! The purpose of the `array_set_optimization` SSA pass is to mark `ArraySet` instructions
//! The purpose of the `mutable_array_set_optimization` SSA pass is to mark `ArraySet` instructions
//! as mutable _iff_ the array is not potentially shared with the callers or callees of the
//! function and won't be used again in the function itself either. In other words, if this
//! is the last time we use this version of the array, we can mutate it in place, and avoid
Expand Down Expand Up @@ -34,29 +34,29 @@ impl Ssa {
/// to do an in-place mutation instead of making a copy if there are
/// no potential shared references to it.
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn array_set_optimization(mut self) -> Self {
pub(crate) fn mutable_array_set_optimization(mut self) -> Self {
for func in self.functions.values_mut() {
#[cfg(debug_assertions)]
array_set_optimization_pre_check(func);
mutable_array_set_optimization_pre_check(func);

func.array_set_optimization();
func.mutable_array_set_optimization();

#[cfg(debug_assertions)]
array_set_optimization_post_check(func);
mutable_array_set_optimization_post_check(func);
}
self
}
}

/// Pre-check condition for [Function::array_set_optimization].
/// Pre-check condition for [Function::mutable_array_set_optimization].
///
/// Only applies to ACIR functions. Panics if:
/// - The function contains more than 1 block, i.e. it hasn't been flattened yet.
/// - There already exists a mutable array set instruction.
/// - There is an `IfElse` instruction which hasn't been removed yet.
/// - There are any Load or Store instructions.
#[cfg(debug_assertions)]
fn array_set_optimization_pre_check(func: &Function) {
fn mutable_array_set_optimization_pre_check(func: &Function) {
// This optimization only applies to ACIR functions
if !func.runtime().is_acir() {
return;
Expand All @@ -74,12 +74,12 @@ fn array_set_optimization_pre_check(func: &Function) {
});
}

/// Post-check condition for [Function::array_set_optimization].
/// Post-check condition for [Function::mutable_array_set_optimization].
///
/// Panics if a Brillig function contains mutable array set instructions.
/// Brillig uses ref-counting to decide whether to mutate an array, not mutable flags.
#[cfg(debug_assertions)]
fn array_set_optimization_post_check(func: &Function) {
fn mutable_array_set_optimization_post_check(func: &Function) {
// Brillig functions should not have any mutable array sets
if func.runtime().is_brillig() {
super::checks::for_each_instruction(func, |instruction, _dfg| {
Expand All @@ -89,7 +89,7 @@ fn array_set_optimization_post_check(func: &Function) {
}

impl Function {
pub(crate) fn array_set_optimization(&mut self) {
pub(crate) fn mutable_array_set_optimization(&mut self) {
if self.runtime().is_brillig() {
// Brillig is supposed to use ref-counting to decide whether to mutate an array;
// array mutation was only meant for ACIR. We could use it with Brillig as well,
Expand Down Expand Up @@ -280,19 +280,7 @@ mod tests {
return v5
}
";
let ssa = Ssa::from_str(src).unwrap();

let ssa = ssa.array_set_optimization();
assert_ssa_snapshot!(ssa, @r"
acir(inline) fn main f0 {
b0():
v1 = make_array [Field 0] : [Field; 1]
v4 = array_set v1, index u32 0, value Field 2
v5 = make_array [v1, v1] : [[Field; 1]; 2]
v6 = array_set v5, index u32 0, value v1
return v6
}
");
assert_ssa_does_not_change(src, Ssa::mutable_array_set_optimization);
}

#[test]
Expand All @@ -312,7 +300,7 @@ mod tests {
return v1
}
";
assert_ssa_does_not_change(src, Ssa::array_set_optimization);
assert_ssa_does_not_change(src, Ssa::mutable_array_set_optimization);
}

#[test]
Expand All @@ -325,7 +313,7 @@ mod tests {
return v1
}
";
assert_ssa_does_not_change(src, Ssa::array_set_optimization);
assert_ssa_does_not_change(src, Ssa::mutable_array_set_optimization);
}

#[test]
Expand All @@ -339,7 +327,7 @@ mod tests {
unreachable
}
";
assert_ssa_does_not_change(src, Ssa::array_set_optimization);
assert_ssa_does_not_change(src, Ssa::mutable_array_set_optimization);
}

// Demonstrate that we assume that `IfElse` instructions have been
Expand All @@ -359,7 +347,7 @@ mod tests {
}
";
let ssa = Ssa::from_str(src).unwrap();
let _ssa = ssa.array_set_optimization();
let _ssa = ssa.mutable_array_set_optimization();
}

#[test]
Expand All @@ -375,7 +363,7 @@ mod tests {
}
";
let ssa = Ssa::from_str(src).unwrap();
let _ssa = ssa.array_set_optimization();
let _ssa = ssa.mutable_array_set_optimization();
}

#[test]
Expand All @@ -391,7 +379,7 @@ mod tests {
}
";
let ssa = Ssa::from_str(src).unwrap();
let _ssa = ssa.array_set_optimization();
let _ssa = ssa.mutable_array_set_optimization();
}

#[test_case("inline")]
Expand All @@ -411,7 +399,7 @@ mod tests {
}}"
);
let ssa = Ssa::from_str(&src).unwrap();
let _ssa = ssa.array_set_optimization();
let _ssa = ssa.mutable_array_set_optimization();
}

// Previously, the first array_set instruction, which modifies v2 in the below
Expand All @@ -426,7 +414,7 @@ mod tests {
return v6, v5
}
";
assert_ssa_does_not_change(src, Ssa::array_set_optimization);
assert_ssa_does_not_change(src, Ssa::mutable_array_set_optimization);
}

#[test]
Expand All @@ -452,7 +440,7 @@ mod tests {
let ssa = Ssa::from_str(src).unwrap();

let (ssa, value) =
assert_pass_does_not_affect_execution(ssa, vec![], Ssa::array_set_optimization);
assert_pass_does_not_affect_execution(ssa, vec![], Ssa::mutable_array_set_optimization);
assert_eq!(value.unwrap()[0], Value::Numeric(NumericValue::Field(0_u32.into())));

assert_ssa_snapshot!(ssa, @r"
Expand Down
Loading