Skip to content

Commit

Permalink
chore: Separate unconstrained functions during monomorphization (#6894)
Browse files Browse the repository at this point in the history
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
  • Loading branch information
jfecher and TomAFrench authored Jan 6, 2025
1 parent da18a12 commit 2e5d91f
Show file tree
Hide file tree
Showing 19 changed files with 240 additions and 493 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,17 @@ pub fn compile_no_check(
cached_program: Option<CompiledProgram>,
force_compile: bool,
) -> Result<CompiledProgram, CompileError> {
let force_unconstrained = options.force_brillig;

let program = if options.instrument_debug {
monomorphize_debug(main_function, &mut context.def_interner, &context.debug_instrumenter)?
monomorphize_debug(
main_function,
&mut context.def_interner,
&context.debug_instrumenter,
force_unconstrained,
)?
} else {
monomorphize(main_function, &mut context.def_interner)?
monomorphize(main_function, &mut context.def_interner, force_unconstrained)?
};

if options.show_monomorphized {
Expand Down Expand Up @@ -632,7 +639,6 @@ pub fn compile_no_check(
}
},
enable_brillig_logging: options.show_brillig,
force_brillig_output: options.force_brillig,
print_codegen_timings: options.benchmark_codegen,
expression_width: if options.bounded_codegen {
options.expression_width.unwrap_or(DEFAULT_EXPRESSION_WIDTH)
Expand Down
29 changes: 9 additions & 20 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ pub struct SsaEvaluatorOptions {

pub enable_brillig_logging: bool,

/// Force Brillig output (for step debugging)
pub force_brillig_output: bool,

/// Pretty print benchmark times of each code generation pass
pub print_codegen_timings: bool,

Expand Down Expand Up @@ -99,7 +96,6 @@ pub(crate) fn optimize_into_acir(
let builder = SsaBuilder::new(
program,
options.ssa_logging.clone(),
options.force_brillig_output,
options.print_codegen_timings,
&options.emit_ssa,
)?;
Expand Down Expand Up @@ -154,15 +150,16 @@ pub(crate) fn optimize_into_acir(
/// Run all SSA passes.
fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result<Ssa, RuntimeError> {
Ok(builder
.run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions")
.run_pass(Ssa::defunctionalize, "Defunctionalization")
.run_pass(Ssa::remove_paired_rc, "Removing Paired rc_inc & rc_decs")
.run_pass(Ssa::separate_runtime, "Runtime Separation")
.run_pass(Ssa::resolve_is_unconstrained, "Resolving IsUnconstrained")
.run_pass(|ssa| ssa.inline_functions(options.inliner_aggressiveness), "Inlining (1st)")
// Run mem2reg with the CFG separated into blocks
.run_pass(Ssa::mem2reg, "Mem2Reg (1st)")
.run_pass(Ssa::simplify_cfg, "Simplifying (1st)")
.run_pass(Ssa::as_slice_optimization, "`as_slice` optimization")
.run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions")
.try_run_pass(
Ssa::evaluate_static_assert_and_assert_constant,
"`static_assert` and `assert_constant`",
Expand Down Expand Up @@ -275,19 +272,12 @@ pub fn create_program(
(generated_acirs, generated_brillig, brillig_function_names, error_types),
ssa_level_warnings,
) = optimize_into_acir(program, options)?;
if options.force_brillig_output {
assert_eq!(
generated_acirs.len(),
1,
"Only the main ACIR is expected when forcing Brillig output"
);
} else {
assert_eq!(
generated_acirs.len(),
func_sigs.len(),
"The generated ACIRs should match the supplied function signatures"
);
}

assert_eq!(
generated_acirs.len(),
func_sigs.len(),
"The generated ACIRs should match the supplied function signatures"
);

let error_types = error_types
.into_iter()
Expand Down Expand Up @@ -450,11 +440,10 @@ impl SsaBuilder {
fn new(
program: Program,
ssa_logging: SsaLogging,
force_brillig_runtime: bool,
print_codegen_timings: bool,
emit_ssa: &Option<PathBuf>,
) -> Result<SsaBuilder, RuntimeError> {
let ssa = ssa_gen::generate_ssa(program, force_brillig_runtime)?;
let ssa = ssa_gen::generate_ssa(program)?;
if let Some(emit_ssa) = emit_ssa {
let mut emit_ssa_dir = emit_ssa.clone();
// We expect the full package artifact path to be passed in here,
Expand Down
44 changes: 24 additions & 20 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,29 +483,33 @@ impl FunctionBuilder {
///
/// Returns whether a reference count instruction was issued.
fn update_array_reference_count(&mut self, value: ValueId, increment: bool) -> bool {
match self.type_of_value(value) {
Type::Numeric(_) => false,
Type::Function => false,
Type::Reference(element) => {
if element.contains_an_array() {
let reference = value;
let value = self.insert_load(reference, element.as_ref().clone());
self.update_array_reference_count(value, increment);
true
} else {
false
if self.current_function.runtime().is_brillig() {
match self.type_of_value(value) {
Type::Numeric(_) => false,
Type::Function => false,
Type::Reference(element) => {
if element.contains_an_array() {
let reference = value;
let value = self.insert_load(reference, element.as_ref().clone());
self.update_array_reference_count(value, increment);
true
} else {
false
}
}
}
Type::Array(..) | Type::Slice(..) => {
// If there are nested arrays or slices, we wait until ArrayGet
// is issued to increment the count of that array.
if increment {
self.insert_inc_rc(value);
} else {
self.insert_dec_rc(value);
Type::Array(..) | Type::Slice(..) => {
// If there are nested arrays or slices, we wait until ArrayGet
// is issued to increment the count of that array.
if increment {
self.insert_inc_rc(value);
} else {
self.insert_dec_rc(value);
}
true
}
true
}
} else {
false
}
}

Expand Down
5 changes: 2 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ fn value(function: &Function, id: ValueId) -> String {
}
Value::Function(id) => id.to_string(),
Value::Intrinsic(intrinsic) => intrinsic.to_string(),
Value::Param { .. } | Value::Instruction { .. } | Value::ForeignFunction(_) => {
id.to_string()
}
Value::ForeignFunction(function) => function.clone(),
Value::Param { .. } | Value::Instruction { .. } => id.to_string(),
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,7 @@ mod test {
fn deduplicates_side_effecting_intrinsics() {
let src = "
// After EnableSideEffectsIf removal:
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: Field, v1: Field, v2: u1):
v4 = call is_unconstrained() -> u1
v7 = call to_be_radix(v0, u32 256) -> [u8; 1] // `a.to_be_radix(256)`;
Expand All @@ -1567,7 +1567,7 @@ mod test {
";
let ssa = Ssa::from_str(src).unwrap();
let expected = "
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: Field, v1: Field, v2: u1):
v4 = call is_unconstrained() -> u1
v7 = call to_be_radix(v0, u32 256) -> [u8; 1]
Expand Down
17 changes: 10 additions & 7 deletions compiler/noirc_evaluator/src/ssa/opt/die.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,10 +637,12 @@ mod test {
use std::sync::Arc;

use im::vector;
use noirc_frontend::monomorphization::ast::InlineType;

use crate::ssa::{
function_builder::FunctionBuilder,
ir::{
function::RuntimeType,
map::Id,
types::{NumericType, Type},
},
Expand Down Expand Up @@ -742,7 +744,7 @@ mod test {
#[test]
fn keep_paired_rcs_with_array_set() {
let src = "
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: [Field; 2]):
inc_rc v0
v2 = array_set v0, index u32 0, value u32 0
Expand All @@ -759,7 +761,7 @@ mod test {

#[test]
fn keep_inc_rc_on_borrowed_array_store() {
// acir(inline) fn main f0 {
// brillig(inline) fn main f0 {
// b0():
// v1 = make_array [u32 0, u32 0]
// v2 = allocate
Expand All @@ -776,6 +778,7 @@ mod test {

// Compiling main
let mut builder = FunctionBuilder::new("main".into(), main_id);
builder.set_runtime(RuntimeType::Brillig(InlineType::Inline));
let zero = builder.numeric_constant(0u128, NumericType::unsigned(32));
let array_type = Type::Array(Arc::new(vec![Type::unsigned(32)]), 2);
let v1 = builder.insert_make_array(vector![zero, zero], array_type.clone());
Expand Down Expand Up @@ -810,7 +813,7 @@ mod test {

#[test]
fn keep_inc_rc_on_borrowed_array_set() {
// acir(inline) fn main f0 {
// brillig(inline) fn main f0 {
// b0(v0: [u32; 2]):
// inc_rc v0
// v3 = array_set v0, index u32 0, value u32 1
Expand All @@ -821,7 +824,7 @@ mod test {
// return v4
// }
let src = "
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: [u32; 2]):
inc_rc v0
v3 = array_set v0, index u32 0, value u32 1
Expand All @@ -837,7 +840,7 @@ mod test {
// We expect the output to be unchanged
// Except for the repeated inc_rc instructions
let expected = "
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: [u32; 2]):
inc_rc v0
v3 = array_set v0, index u32 0, value u32 1
Expand Down Expand Up @@ -875,7 +878,7 @@ mod test {
#[test]
fn remove_inc_rcs_that_are_never_mutably_borrowed() {
let src = "
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: [Field; 2]):
inc_rc v0
inc_rc v0
Expand All @@ -893,7 +896,7 @@ mod test {
assert_eq!(main.dfg[main.entry_block()].instructions().len(), 5);

let expected = "
acir(inline) fn main f0 {
brillig(inline) fn main f0 {
b0(v0: [Field; 2]):
v2 = array_get v0, index u32 0 -> Field
return v2
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ mod tests {
let options = &SsaEvaluatorOptions {
ssa_logging: SsaLogging::None,
enable_brillig_logging: false,
force_brillig_output: false,
print_codegen_timings: false,
expression_width: ExpressionWidth::default(),
emit_ssa: None,
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 @@ -20,8 +20,8 @@ mod rc;
mod remove_bit_shifts;
mod remove_enable_side_effects;
mod remove_if_else;
mod remove_unreachable;
mod resolve_is_unconstrained;
mod runtime_separation;
mod simplify_cfg;
mod unrolling;

Expand Down
4 changes: 3 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ impl IdMaps {
}

Value::Function(id) => {
let new_id = self.function_ids[id];
let new_id = *self.function_ids.get(id).unwrap_or_else(|| {
unreachable!("Unmapped function with id {id}")
});
new_function.dfg.import_function(new_id)
}

Expand Down
80 changes: 80 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/remove_unreachable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
use std::collections::BTreeSet;

use fxhash::FxHashSet as HashSet;

use crate::ssa::{
ir::{
function::{Function, FunctionId},
instruction::Instruction,
value::Value,
},
ssa_gen::Ssa,
};

impl Ssa {
/// Removes any unreachable functions from the code. These can result from
/// optimizations making existing functions unreachable, e.g. `if false { foo() }`,
/// or even from monomorphizing an unconstrained version of a constrained function
/// where the original constrained version ends up never being used.
pub(crate) fn remove_unreachable_functions(mut self) -> Self {
let mut used_functions = HashSet::default();

for function_id in self.functions.keys() {
if self.is_entry_point(*function_id) {
collect_reachable_functions(&self, *function_id, &mut used_functions);
}
}

self.functions.retain(|id, _| used_functions.contains(id));
self
}
}

fn collect_reachable_functions(
ssa: &Ssa,
current_func_id: FunctionId,
reachable_functions: &mut HashSet<FunctionId>,
) {
if reachable_functions.contains(&current_func_id) {
return;
}
reachable_functions.insert(current_func_id);

// If the debugger is used, its possible for function inlining
// to remove functions that the debugger still references
let Some(func) = ssa.functions.get(&current_func_id) else {
return;
};

let used_functions = used_functions(func);

for called_func_id in used_functions.iter() {
collect_reachable_functions(ssa, *called_func_id, reachable_functions);
}
}

fn used_functions(func: &Function) -> BTreeSet<FunctionId> {
let mut used_function_ids = BTreeSet::default();

let mut find_functions = |value| {
if let Value::Function(function) = func.dfg[func.dfg.resolve(value)] {
used_function_ids.insert(function);
}
};

for block_id in func.reachable_blocks() {
let block = &func.dfg[block_id];

for instruction_id in block.instructions() {
let instruction = &func.dfg[*instruction_id];

if matches!(instruction, Instruction::Store { .. } | Instruction::Call { .. }) {
instruction.for_each_value(&mut find_functions);
}
}

block.unwrap_terminator().for_each_value(&mut find_functions);
}

used_function_ids
}
Loading

0 comments on commit 2e5d91f

Please sign in to comment.