Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Separate unconstrained functions during monomorphization #6894

Merged
merged 20 commits into from
Jan 6, 2025
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
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 {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -469,29 +469,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 @@ -180,7 +180,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 { .. }) {
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
instruction.for_each_value(&mut find_functions);
}
}

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

used_function_ids
}
Loading
Loading