Skip to content

Commit

Permalink
fix(ssa): Only attempt to inline constant Brillig calls for entry poi…
Browse files Browse the repository at this point in the history
…nts (#7260)
  • Loading branch information
vezenovm authored Feb 3, 2025
1 parent a9e9850 commit 1ae035f
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 12 deletions.
6 changes: 5 additions & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ pub(crate) fn gen_brillig_for(
brillig: &Brillig,
) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
// Create the entry point artifact
let globals_memory_size = brillig.globals_memory_size.get(&func.id()).copied().unwrap_or(0);
let globals_memory_size = brillig
.globals_memory_size
.get(&func.id())
.copied()
.expect("Should have the globals memory size specified for an entry point");
let mut entry_point = BrilligContext::new_entry_point_artifact(
arguments,
FunctionContext::return_values(func),
Expand Down
105 changes: 94 additions & 11 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ impl Ssa {
let func_value = &function.dfg[*func_id];
let Value::Function(func_id) = func_value else { continue };

brillig_functions.remove(func_id);
if function.runtime().is_acir() {
brillig_functions.remove(func_id);
}
}
}
}
Expand Down Expand Up @@ -336,17 +338,22 @@ impl<'brillig> Context<'brillig> {
};

// First try to inline a call to a brillig function with all constant arguments.
let new_results = Self::try_inline_brillig_call_with_all_constants(
&instruction,
&old_results,
block,
dfg,
self.brillig_info,
)
// Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs.
.unwrap_or_else(|| {
let new_results = if runtime_is_brillig {
Self::push_instruction(id, instruction.clone(), &old_results, block, dfg)
});
} else {
// We only want to try to inline Brillig calls for Brillig entry points (functions called from an ACIR runtime).
Self::try_inline_brillig_call_with_all_constants(
&instruction,
&old_results,
block,
dfg,
self.brillig_info,
)
// Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs.
.unwrap_or_else(|| {
Self::push_instruction(id, instruction.clone(), &old_results, block, dfg)
})
};

Self::replace_result_ids(dfg, &old_results, &new_results);

Expand Down Expand Up @@ -1485,6 +1492,82 @@ mod test {
assert_normalized_ssa_equals(ssa, expected);
}

#[test]
fn inlines_brillig_call_with_entry_point_globals() {
let src = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
v1 = call f1() -> Field
return v1
}
brillig(inline) fn one f1 {
b0():
v1 = add g0, Field 3
return v1
}
";
let ssa = Ssa::from_str(src).unwrap();
let mut ssa = ssa.dead_instruction_elimination();
let used_globals_map = std::mem::take(&mut ssa.used_globals);
let brillig = ssa.to_brillig_with_globals(false, used_globals_map);

let expected = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
return Field 5
}
";

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

#[test]
fn inlines_brillig_call_with_non_entry_point_globals() {
let src = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
v1 = call f1() -> Field
return v1
}
brillig(inline) fn entry_point f1 {
b0():
v1 = call f2() -> Field
return v1
}
brillig(inline) fn one f2 {
b0():
v1 = add g0, Field 3
return v1
}
";
let ssa = Ssa::from_str(src).unwrap();
let mut ssa = ssa.dead_instruction_elimination();
let used_globals_map = std::mem::take(&mut ssa.used_globals);
let brillig = ssa.to_brillig_with_globals(false, used_globals_map);

let expected = "
g0 = Field 2
acir(inline) fn main f0 {
b0():
return Field 5
}
";

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

#[test]
fn does_not_use_cached_constrain_in_block_that_is_not_dominated() {
let src = "
Expand Down

1 comment on commit 1ae035f

@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: 1ae035f Previous: a9e9850 Ratio
noir-lang_schnorr_ 1 s 0 s +∞

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

CC: @TomAFrench

Please sign in to comment.