Skip to content

Commit

Permalink
Merge branch 'master' into 6831-strip-rc
Browse files Browse the repository at this point in the history
* master:
  chore: Separate unconstrained functions during monomorphization (#6894)
  feat!: turn CannotReexportItemWithLessVisibility into an error (#6952)
  feat: lock on Nargo.toml on several nargo commands (#6941)
  • Loading branch information
TomAFrench committed Jan 6, 2025
2 parents a5f6157 + 2e5d91f commit d5a6c78
Show file tree
Hide file tree
Showing 24 changed files with 302 additions and 536 deletions.
23 changes: 12 additions & 11 deletions Cargo.lock

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

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ noir_lsp = { path = "tooling/lsp" }
noir_debugger = { path = "tooling/debugger" }
noirc_abi = { path = "tooling/noirc_abi" }
noirc_artifacts = { path = "tooling/noirc_artifacts" }
bb_abstraction_leaks = { path = "tooling/bb_abstraction_leaks" }
acvm_cli = { path = "tooling/acvm_cli" }

# Arkworks
ark-bn254 = { version = "^0.5.0", default-features = false, features = [
Expand Down
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 @@ -484,29 +484,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
Loading

0 comments on commit d5a6c78

Please sign in to comment.