Skip to content

Commit

Permalink
fix(ssa): Unused functions removals post folding constant Brillig cal…
Browse files Browse the repository at this point in the history
…ls (#7265)
  • Loading branch information
vezenovm authored Feb 4, 2025
1 parent 130d991 commit d5d6cb7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 43 deletions.
3 changes: 3 additions & 0 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ pub(crate) fn optimize_into_acir(
print_codegen_timings: options.print_codegen_timings,
}
.run_pass(|ssa| ssa.fold_constants_with_brillig(&brillig), "Inlining Brillig Calls Inlining")
// It could happen that we inlined all calls to a given brillig function.
// In that case it's unused so we can remove it. This is what we check next.
.run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions (3rd)")
.run_pass(Ssa::dead_instruction_elimination, "Dead Instruction Elimination (2nd)")
.finish();

Expand Down
51 changes: 8 additions & 43 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,49 +98,6 @@ impl Ssa {
function.constant_fold(false, brillig_info);
}

// It could happen that we inlined all calls to a given brillig function.
// In that case it's unused so we can remove it. This is what we check next.
self.remove_unused_brillig_functions(brillig_functions)
}

fn remove_unused_brillig_functions(
mut self,
mut brillig_functions: BTreeMap<FunctionId, Function>,
) -> Ssa {
// Remove from the above map functions that are called
for function in self.functions.values() {
for block_id in function.reachable_blocks() {
for instruction_id in function.dfg[block_id].instructions() {
let instruction = &function.dfg[*instruction_id];
let Instruction::Call { func: func_id, arguments: _ } = instruction else {
continue;
};

let func_value = &function.dfg[*func_id];
let Value::Function(func_id) = func_value else { continue };

if function.runtime().is_acir() {
brillig_functions.remove(func_id);
}
}
}
}

// The ones that remain are never called: let's remove them.
for (func_id, func) in &brillig_functions {
// We never want to remove the main function (it could be `unconstrained` or it
// could have been turned into brillig if `--force-brillig` was given).
// We also don't want to remove entry points.
let runtime = func.runtime();
if self.main_id == *func_id
|| (runtime.is_entry_point() && matches!(runtime, RuntimeType::Acir(_)))
{
continue;
}

self.functions.remove(func_id);
}

self
}
}
Expand Down Expand Up @@ -1346,6 +1303,7 @@ mod test {
}
";
let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1374,6 +1332,7 @@ mod test {
}
";
let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1402,6 +1361,7 @@ mod test {
}
";
let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1431,6 +1391,7 @@ mod test {
}
";
let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1460,6 +1421,7 @@ mod test {
}
";
let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1494,6 +1456,7 @@ mod test {
}
";
let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1529,6 +1492,7 @@ mod test {
";

let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down Expand Up @@ -1570,6 +1534,7 @@ mod test {
";

let ssa = ssa.fold_constants_with_brillig(&brillig);
let ssa = ssa.remove_unreachable_functions();
assert_normalized_ssa_equals(ssa, expected);
}

Expand Down

3 comments on commit d5d6cb7

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d5d6cb7 Previous: 130d991 Ratio
noir-lang_mimc_ 1 s 0 s +∞
noir-lang_eddsa_ 2 s 1 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d5d6cb7 Previous: 130d991 Ratio
sha256_regression 0.069 s 0.051 s 1.35

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d5d6cb7 Previous: 130d991 Ratio
noir-lang_noir_check_shuffle_ 1 s 0 s +∞
noir-lang_mimc_ 1 s 0 s +∞
noir-lang_ec_ 1 s 0 s +∞

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Please sign in to comment.