Skip to content

Commit 4a64541

Browse files
committed
Don't use new function if it's not better than the old one
1 parent 779a4d8 commit 4a64541

File tree

2 files changed

+50
-9
lines changed

2 files changed

+50
-9
lines changed

compiler/noirc_evaluator/src/ssa/opt/inline_constants_into_brillig_functions.rs

+39-9
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,20 @@ impl Ssa {
5959

6060
for (func_id, entries) in calls {
6161
let function = self.functions[&func_id].clone();
62+
let function_num_instructions = function.num_instructions();
6263
for (constants, _) in entries {
63-
let new_function_id = self.add_fn(|func_id| {
64-
inline_constants_into_function(&function, &constants, func_id)
65-
});
64+
let Some(new_function_id) = self.maybe_add_fn(|func_id| {
65+
let new_function =
66+
inline_constants_into_function(&function, &constants, func_id);
67+
// No point in using the new function if it's not more optimal
68+
if new_function.num_instructions() < function_num_instructions {
69+
Some(new_function)
70+
} else {
71+
None
72+
}
73+
}) else {
74+
continue;
75+
};
6676
let entry = new_functions.entry(func_id).or_default();
6777
entry.entry(constants).insert_entry(new_function_id);
6878
}
@@ -287,8 +297,8 @@ mod tests {
287297
}
288298
brillig(inline) fn foo f1 {
289299
b0(v0: Field, v1: Field):
290-
v2 = add v0, v1
291-
return v2
300+
v3 = add v0, Field 1
301+
return v3
292302
}
293303
";
294304
let ssa = Ssa::from_str(src).unwrap();
@@ -303,13 +313,12 @@ mod tests {
303313
}
304314
brillig(inline) fn foo f1 {
305315
b0(v0: Field, v1: Field):
306-
v2 = add v0, v1
307-
return v2
316+
v3 = add v0, Field 1
317+
return v3
308318
}
309319
brillig(inline) fn foo f2 {
310320
b0(v0: Field):
311-
v2 = add Field 1, v0
312-
return v2
321+
return Field 2
313322
}
314323
";
315324
let ssa = ssa.inline_constants_into_brillig_functions();
@@ -363,6 +372,27 @@ mod tests {
363372
assert_normalized_ssa_equals(ssa, expected);
364373
}
365374

375+
#[test]
376+
fn does_not_inline_if_inlined_function_does_not_have_less_instructions() {
377+
let src = "
378+
acir(inline) fn main f0 {
379+
b0(v0: Field):
380+
v3 = call f1(Field 1, v0) -> Field
381+
v4 = call f1(Field 1, v0) -> Field
382+
v5 = add v3, v4
383+
return v5
384+
}
385+
brillig(inline) fn foo f1 {
386+
b0(v0: Field, v1: Field):
387+
v2 = add v0, v1
388+
return v2
389+
}
390+
";
391+
let ssa = Ssa::from_str(src).unwrap();
392+
let ssa = ssa.inline_constants_into_brillig_functions();
393+
assert_normalized_ssa_equals(ssa, src);
394+
}
395+
366396
#[test]
367397
fn does_not_inline_brillig_call_into_brillig_function() {
368398
let src = "

compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs

+11
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,17 @@ impl Ssa {
8585
new_id
8686
}
8787

88+
/// Adds a new function to the program, but only if the given lambda returns `Some`.
89+
pub(crate) fn maybe_add_fn(
90+
&mut self,
91+
build_with_id: impl FnOnce(FunctionId) -> Option<Function>,
92+
) -> Option<FunctionId> {
93+
let new_id = self.next_id.next();
94+
let function = build_with_id(new_id)?;
95+
self.functions.insert(new_id, function);
96+
Some(new_id)
97+
}
98+
8899
pub(crate) fn generate_entry_point_index(mut self) -> Self {
89100
let entry_points =
90101
self.functions.keys().filter(|function| self.is_entry_point(**function)).enumerate();

0 commit comments

Comments
 (0)