From a2180ff6f5b1ba520ca063fd731dac2415bc8ced Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 6 Dec 2024 15:17:01 +0000 Subject: [PATCH 1/4] chore: pull across macro ordering changes --- .../noirc_frontend/src/elaborator/comptime.rs | 154 ++++++++++-------- .../noirc_frontend/src/elaborator/mod.rs | 12 +- .../noirc_frontend/src/elaborator/types.rs | 2 +- .../src/hir/def_collector/dc_crate.rs | 3 +- .../noirc_frontend/src/hir/def_map/mod.rs | 23 +++ .../src/hir/def_map/module_data.rs | 10 ++ 6 files changed, 123 insertions(+), 81 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs index 962356d6dd9..65d98dbf678 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -39,6 +39,8 @@ struct AttributeContext { attribute_module: LocalModuleId, } +type CollectedAttributes = Vec<(FuncId, Value, Vec, AttributeContext, Span)>; + impl AttributeContext { fn new(file: FileId, module: LocalModuleId) -> Self { Self { file, module, attribute_file: file, attribute_module: module } @@ -131,41 +133,37 @@ impl<'context> Elaborator<'context> { } } - fn run_comptime_attributes_on_item( + fn collect_comptime_attributes_on_item( &mut self, attributes: &[SecondaryAttribute], item: Value, - span: Span, attribute_context: AttributeContext, - generated_items: &mut CollectedItems, + attributes_to_run: &mut CollectedAttributes, ) { for attribute in attributes { - self.run_comptime_attribute_on_item( + self.collect_comptime_attribute_on_item( attribute, &item, - span, attribute_context, - generated_items, + attributes_to_run, ); } } - fn run_comptime_attribute_on_item( + fn collect_comptime_attribute_on_item( &mut self, attribute: &SecondaryAttribute, item: &Value, - span: Span, attribute_context: AttributeContext, - generated_items: &mut CollectedItems, + attributes_to_run: &mut CollectedAttributes, ) { if let SecondaryAttribute::Meta(attribute) = attribute { self.elaborate_in_comptime_context(|this| { - if let Err(error) = this.run_comptime_attribute_name_on_item( + if let Err(error) = this.collect_comptime_attribute_name_on_item( attribute, item.clone(), - span, attribute_context, - generated_items, + attributes_to_run, ) { this.errors.push(error); } @@ -173,22 +171,18 @@ impl<'context> Elaborator<'context> { } } - fn run_comptime_attribute_name_on_item( + fn collect_comptime_attribute_name_on_item( &mut self, attribute: &MetaAttribute, item: Value, - span: Span, attribute_context: AttributeContext, - generated_items: &mut CollectedItems, + attributes_to_run: &mut CollectedAttributes, ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.attribute_file; self.local_module = attribute_context.attribute_module; + let span = attribute.span; - let location = Location::new(attribute.span, self.file); - let function = Expression { - kind: ExpressionKind::Variable(attribute.name.clone()), - span: attribute.span, - }; + let function = Expression { kind: ExpressionKind::Variable(attribute.name.clone()), span }; let arguments = attribute.arguments.clone(); // Elaborate the function, rolling back any errors generated in case it is unknown @@ -200,32 +194,35 @@ impl<'context> Elaborator<'context> { let definition_id = match self.interner.expression(&function) { HirExpression::Ident(ident, _) => ident.id, _ => { - return Err(( - ResolverError::AttributeFunctionIsNotAPath { - function: function_string, - span: attribute.span, - } - .into(), - self.file, - )) + let error = + ResolverError::AttributeFunctionIsNotAPath { function: function_string, span }; + return Err((error.into(), self.file)); } }; let Some(definition) = self.interner.try_definition(definition_id) else { - return Err(( - ResolverError::AttributeFunctionNotInScope { - name: function_string, - span: attribute.span, - } - .into(), - self.file, - )); + let error = ResolverError::AttributeFunctionNotInScope { name: function_string, span }; + return Err((error.into(), self.file)); }; let DefinitionKind::Function(function) = definition.kind else { return Err((ResolverError::NonFunctionInAnnotation { span }.into(), self.file)); }; + attributes_to_run.push((function, item, arguments, attribute_context, span)); + Ok(()) + + } + + fn run_attribute( + &mut self, + attribute_context: AttributeContext, + function: FuncId, + arguments: Vec, + item: Value, + location: Location, + generated_items: &mut CollectedItems, + ) -> Result<(), (CompilationError, FileId)> { self.file = attribute_context.file; self.local_module = attribute_context.module; @@ -237,10 +234,7 @@ impl<'context> Elaborator<'context> { arguments, location, ) - .map_err(|error| { - let file = error.get_location().file; - (error.into(), file) - })?; + .map_err(|error| error.into_compilation_error_pair())?; arguments.insert(0, (item, location)); @@ -496,65 +490,91 @@ impl<'context> Elaborator<'context> { } } - /// Run all the attributes on each item. The ordering is unspecified to users but currently - /// we run trait attributes first to (e.g.) register derive handlers before derive is - /// called on structs. - /// Returns any new items generated by attributes. + /// Run all the attributes on each item in the crate in source-order. + /// Source-order is defined as running all child modules before their parent modules are run. + /// Child modules of a parent are run in order of their `mod foo;` declarations in the parent. pub(super) fn run_attributes( &mut self, traits: &BTreeMap, types: &BTreeMap, functions: &[UnresolvedFunctions], module_attributes: &[ModuleAttribute], - ) -> CollectedItems { - let mut generated_items = CollectedItems::default(); + ) { + let mut attributes_to_run = Vec::new(); for (trait_id, trait_) in traits { let attributes = &trait_.trait_def.attributes; let item = Value::TraitDefinition(*trait_id); - let span = trait_.trait_def.span; let context = AttributeContext::new(trait_.file_id, trait_.module_id); - self.run_comptime_attributes_on_item( + self.collect_comptime_attributes_on_item( attributes, item, - span, context, - &mut generated_items, + &mut attributes_to_run, ); } for (struct_id, struct_def) in types { let attributes = &struct_def.struct_def.attributes; let item = Value::StructDefinition(*struct_id); - let span = struct_def.struct_def.span; let context = AttributeContext::new(struct_def.file_id, struct_def.module_id); - self.run_comptime_attributes_on_item( + self.collect_comptime_attributes_on_item( attributes, item, - span, context, - &mut generated_items, + &mut attributes_to_run, ); } - self.run_attributes_on_functions(functions, &mut generated_items); + self.collect_attributes_on_functions(functions, &mut attributes_to_run); + self.collect_attributes_on_modules(module_attributes, &mut attributes_to_run); + + self.sort_attributes_by_run_order(&mut attributes_to_run); + + // run + for (attribute, item, args, context, span) in attributes_to_run { + let location = Location::new(span, context.attribute_file); + + let mut generated_items = CollectedItems::default(); + self.elaborate_in_comptime_context(|this| { + if let Err(error) = this.run_attribute( + context, + attribute, + args, + item, + location, + &mut generated_items, + ) { + this.errors.push(error); + } + }); + + if !generated_items.is_empty() { + self.elaborate_items(generated_items); + } + } + } - self.run_attributes_on_modules(module_attributes, &mut generated_items); + fn sort_attributes_by_run_order(&self, attributes: &mut CollectedAttributes) { + let module_order = self.def_maps[&self.crate_id].get_module_topological_order(); - generated_items + // Sort each attribute by (module, location in file) so that we can execute in + // the order they were defined in, running attributes in child modules first. + attributes.sort_by_key(|(_, _, _, ctx, span)| { + (module_order[&ctx.attribute_module], span.start()) + }); } - fn run_attributes_on_modules( + fn collect_attributes_on_modules( &mut self, module_attributes: &[ModuleAttribute], - generated_items: &mut CollectedItems, - ) { + attributes_to_run: &mut CollectedAttributes, + ) { for module_attribute in module_attributes { let local_id = module_attribute.module_id; let module_id = ModuleId { krate: self.crate_id, local_id }; let item = Value::ModuleDefinition(module_id); let attribute = &module_attribute.attribute; - let span = Span::default(); let context = AttributeContext { file: module_attribute.file_id, @@ -563,14 +583,14 @@ impl<'context> Elaborator<'context> { attribute_module: module_attribute.attribute_module_id, }; - self.run_comptime_attribute_on_item(attribute, &item, span, context, generated_items); + self.collect_comptime_attribute_on_item(attribute, &item, context, attributes_to_run); } } - fn run_attributes_on_functions( + fn collect_attributes_on_functions( &mut self, function_sets: &[UnresolvedFunctions], - generated_items: &mut CollectedItems, + attributes_to_run: &mut CollectedAttributes, ) { for function_set in function_sets { self.self_type = function_set.self_type.clone(); @@ -579,13 +599,11 @@ impl<'context> Elaborator<'context> { let context = AttributeContext::new(function_set.file_id, *local_module); let attributes = function.secondary_attributes(); let item = Value::FunctionDefinition(*function_id); - let span = function.span(); - self.run_comptime_attributes_on_item( + self.collect_comptime_attributes_on_item( attributes, item, - span, context, - generated_items, + attributes_to_run, ); } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index 478504a79be..fe1d1e38e1a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -307,23 +307,13 @@ impl<'context> Elaborator<'context> { // We have to run any comptime attributes on functions before the function is elaborated // since the generated items are checked beforehand as well. - let generated_items = self.run_attributes( + self.run_attributes( &items.traits, &items.types, &items.functions, &items.module_attributes, ); - // After everything is collected, we can elaborate our generated items. - // It may be better to inline these within `items` entirely since elaborating them - // all here means any globals will not see these. Inlining them completely within `items` - // means we must be more careful about missing any additional items that need to be already - // elaborated. E.g. if a new struct is created, we've already passed the code path to - // elaborate them. - if !generated_items.is_empty() { - self.elaborate_items(generated_items); - } - for functions in items.functions { self.elaborate_functions(functions); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index 0404ae3c2c0..2e4809f3511 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -576,7 +576,7 @@ impl<'context> Elaborator<'context> { fn resolve_trait_static_method(&mut self, path: &Path) -> Option { let path_resolution = self.resolve_path(path.clone()).ok()?; let func_id = path_resolution.item.function_id()?; - let meta = self.interner.function_meta(&func_id); + let meta = self.interner.try_function_meta(&func_id)?; let the_trait = self.interner.get_trait(meta.trait_id?); let method = the_trait.find_method(path.last_name())?; let constraint = the_trait.as_constraint(path.span); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 51e62599b05..7eb4db43ee1 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -119,8 +119,9 @@ pub struct ModuleAttribute { pub file_id: FileId, // The module this attribute is attached to pub module_id: LocalModuleId, + // The file where the attribute exists (it could be the same as `file_id` - // or a different one if it's an inner attribute in a different file) + // or a different one if it is an outer attribute in the parent of the module it applies to) pub attribute_file_id: FileId, // The module where the attribute is defined (similar to `attribute_file_id`, // it could be different than `module_id` for inner attributes) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs index 3bb16a92fdb..d9d6e150a7a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -289,6 +289,29 @@ impl CrateDefMap { String::new() } } + + /// Return a topological ordering of each module such that any child modules + /// are before their parent modules. Sibling modules will respect the ordering + /// declared from their parent module (the `mod foo; mod bar;` declarations). + pub fn get_module_topological_order(&self) -> HashMap { + let mut ordering = HashMap::default(); + self.topologically_sort_modules(self.root, &mut 0, &mut ordering); + ordering + } + + fn topologically_sort_modules( + &self, + current: LocalModuleId, + index: &mut usize, + ordering: &mut HashMap, + ) { + for child in &self.modules[current.0].child_declaration_order { + self.topologically_sort_modules(*child, index, ordering); + } + + ordering.insert(current, *index); + *index += 1; + } } /// Specifies a contract function and extra metadata that diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs index fe6fe8285d3..06188f3920b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -14,6 +14,11 @@ pub struct ModuleData { pub parent: Option, pub children: HashMap, + /// Each child in the order they were declared in the parent module. + /// E.g. for a module containing `mod foo; mod bar; mod baz` this would + /// be `vec![foo, bar, baz]`. + pub child_declaration_order: Vec, + /// Contains all definitions visible to the current module. This includes /// all definitions in self.definitions as well as all imported definitions. scope: ItemScope, @@ -47,6 +52,7 @@ impl ModuleData { ModuleData { parent, children: HashMap::new(), + child_declaration_order: Vec::new(), scope: ItemScope::default(), definitions: ItemScope::default(), location, @@ -73,6 +79,10 @@ impl ModuleData { ) -> Result<(), (Ident, Ident)> { self.scope.add_definition(name.clone(), visibility, item_id, trait_id)?; + if let ModuleDefId::ModuleId(child) = item_id { + self.child_declaration_order.push(child.local_id); + } + // definitions is a subset of self.scope so it is expected if self.scope.define_func_def // returns without error, so will self.definitions.define_func_def. self.definitions.add_definition(name, visibility, item_id, trait_id) From f0fd956a558600666ceec1b833ef0711fc2ee65a Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 6 Dec 2024 15:33:49 +0000 Subject: [PATCH 2/4] . --- .../compiler/noirc_frontend/src/elaborator/comptime.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs index 65d98dbf678..076305a3165 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -211,7 +211,6 @@ impl<'context> Elaborator<'context> { attributes_to_run.push((function, item, arguments, attribute_context, span)); Ok(()) - } fn run_attribute( @@ -569,7 +568,7 @@ impl<'context> Elaborator<'context> { &mut self, module_attributes: &[ModuleAttribute], attributes_to_run: &mut CollectedAttributes, - ) { + ) { for module_attribute in module_attributes { let local_id = module_attribute.module_id; let module_id = ModuleId { krate: self.crate_id, local_id }; @@ -583,7 +582,7 @@ impl<'context> Elaborator<'context> { attribute_module: module_attribute.attribute_module_id, }; - self.collect_comptime_attribute_on_item(attribute, &item, context, attributes_to_run); + self.collect_comptime_attribute_on_item(attribute, &item, context, attributes_to_run); } } From 011d2cf0eb738718119159fb44a6e82f52a63846 Mon Sep 17 00:00:00 2001 From: thunkar Date: Mon, 9 Dec 2024 09:35:53 +0000 Subject: [PATCH 3/4] updated events definition --- avm-transpiler/Cargo.lock | 1 - noir-projects/aztec-nr/aztec/src/macros/events/mod.nr | 2 +- .../contract_instance_deployer_contract/src/main.nr | 2 +- .../contracts/crowdfunding_contract/src/main.nr | 2 +- .../noir-contracts/contracts/nft_contract/src/main.nr | 4 ++-- .../noir-contracts/contracts/test_contract/src/main.nr | 5 +---- .../contracts/test_log_contract/src/main.nr | 9 ++------- 7 files changed, 8 insertions(+), 17 deletions(-) diff --git a/avm-transpiler/Cargo.lock b/avm-transpiler/Cargo.lock index bdac1771a70..4da74e41190 100644 --- a/avm-transpiler/Cargo.lock +++ b/avm-transpiler/Cargo.lock @@ -948,7 +948,6 @@ dependencies = [ "acvm", "iter-extended", "jsonrpc", - "regex", "serde", "serde_json", "thiserror", diff --git a/noir-projects/aztec-nr/aztec/src/macros/events/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/events/mod.nr index 3a72031efc0..1fc3671dd7e 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/events/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/events/mod.nr @@ -1,5 +1,6 @@ use super::utils::compute_event_selector; use protocol_types::meta::flatten_to_fields; +use std::meta::typ::fresh_type_variable; comptime fn generate_event_interface(s: StructDefinition) -> Quoted { let name = s.name(); @@ -13,7 +14,6 @@ comptime fn generate_event_interface(s: StructDefinition) -> Quoted { impl aztec::event::event_interface::EventInterface<$content_len> for $name { fn to_be_bytes(self) -> [u8; $content_len * 32 + 32] { let mut buffer: [u8; $content_len * 32 + 32] = [0; $content_len * 32 + 32]; - let event_type_id_bytes: [u8; 32] = $name::get_event_type_id().to_field().to_be_bytes(); for i in 0..32 { diff --git a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr index 04ef191bc9f..d686ee96cb3 100644 --- a/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/contract_instance_deployer_contract/src/main.nr @@ -13,8 +13,8 @@ contract ContractInstanceDeployer { }; use std::meta::derive; - #[event] #[derive(Serialize)] + #[event] struct ContractInstanceDeployed { DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE: Field, address: AztecAddress, diff --git a/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr b/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr index 40d738a512c..b4e2a0e0b27 100644 --- a/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/crowdfunding_contract/src/main.nr @@ -27,8 +27,8 @@ contract Crowdfunding { use token::Token; // docs:end:all-deps - #[event] #[derive(Serialize)] + #[event] struct WithdrawalProcessed { who: AztecAddress, amount: u64, diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index ecec8c4a226..e5f76dc31ce 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -30,12 +30,12 @@ contract NFT { utils::comparison::Comparator, }; use dep::compressed_string::FieldCompressedString; - use std::{embedded_curve_ops::EmbeddedCurvePoint, meta::derive}; + use std::meta::derive; // TODO(#8467): Rename this to Transfer - calling this NFTTransfer to avoid export conflict with the Transfer event // in the Token contract. - #[event] #[derive(Serialize)] + #[event] struct NFTTransfer { from: AztecAddress, to: AztecAddress, diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr index 30210cbdadb..36cb5e8375a 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -39,14 +39,11 @@ contract Test { get_mint_to_private_content_hash, get_mint_to_public_content_hash, }; use dep::value_note::value_note::ValueNote; - // TODO investigate why the macros require EmbeddedCurvePoint and EmbeddedCurveScalar - use std::embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar}; use std::meta::derive; use crate::test_note::TestNote; - - #[event] #[derive(Serialize)] + #[event] struct ExampleEvent { value0: Field, value1: Field, diff --git a/noir-projects/noir-contracts/contracts/test_log_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_log_contract/src/main.nr index fa7c33be0b3..85e57131c20 100644 --- a/noir-projects/noir-contracts/contracts/test_log_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_log_contract/src/main.nr @@ -11,17 +11,15 @@ contract TestLog { use dep::value_note::value_note::ValueNote; use std::meta::derive; - use std::embedded_curve_ops::EmbeddedCurveScalar; - - #[event] #[derive(Serialize)] + #[event] struct ExampleEvent0 { value0: Field, value1: Field, } - #[event] #[derive(Serialize)] + #[event] struct ExampleEvent1 { value2: AztecAddress, value3: u8, @@ -32,9 +30,6 @@ contract TestLog { example_set: PrivateSet, } - // EXAMPLE_EVENT_0_BYTES_LEN + 16 - global EXAMPLE_EVENT_0_CIPHERTEXT_BYTES_LEN: Field = 144; - #[private] fn emit_encrypted_events(other: AztecAddress, preimages: [Field; 4]) { let event0 = ExampleEvent0 { value0: preimages[0], value1: preimages[1] }; From 7fa8f844d9e389f8636118f1d3c3ecce707e771e Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 9 Dec 2024 09:53:47 +0000 Subject: [PATCH 4/4] chore: Optimise grand product computation round based on active ranges (#10460) Skip computation of numerator and denominator of z_perm at indexes that are not part of the active ranges, and refine the threshold in commit_structured for `z_perm`. New benchmarks: **Now: 5.6 s difference** We still see a difference between committing to z_perm between an ambient trace of 2^19 and 2^20, caused by the fact that the active ranges complement are larger (i.e. the ranges in the trace blocks where z_perm is constant) because the blocks themselves are larger. We make sure to at least avoid computing and committing to z_perm after the final active wire index. --- .../commitment_schemes/commitment_key.hpp | 59 ++++++++++--------- .../batched_affine_addition.cpp | 1 + .../barretenberg/ecc/fields/field_impl.hpp | 3 +- .../cpp/src/barretenberg/flavor/flavor.hpp | 2 +- .../execution_trace_usage_tracker.hpp | 1 + .../library/grand_product_library.hpp | 36 +++++++---- .../relations/databus_lookup_relation.hpp | 1 + .../relations/logderiv_lookup_relation.hpp | 1 + .../stdlib_circuit_builders/mega_flavor.hpp | 4 +- .../stdlib_circuit_builders/ultra_flavor.hpp | 2 +- .../trace_to_polynomials.cpp | 1 + .../barretenberg/ultra_honk/oink_prover.cpp | 5 +- .../barretenberg/ultra_honk/oink_prover.hpp | 7 ++- 13 files changed, 79 insertions(+), 44 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 6440c016a62..81ef54a2b9d 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -85,7 +85,7 @@ template class CommitmentKey { */ Commitment commit(PolynomialSpan polynomial) { - PROFILE_THIS(); + PROFILE_THIS_NAME("commit"); // We must have a power-of-2 SRS points *after* subtracting by start_index. size_t dyadic_poly_size = numeric::round_up_power_2(polynomial.size()); // Because pippenger prefers a power-of-2 size, we must choose a starting index for the points so that we don't @@ -133,7 +133,7 @@ template class CommitmentKey { */ Commitment commit_sparse(PolynomialSpan polynomial) { - PROFILE_THIS(); + PROFILE_THIS_NAME("commit_sparse"); const size_t poly_size = polynomial.size(); ASSERT(polynomial.end_index() <= srs->get_monomial_size()); @@ -204,21 +204,24 @@ template class CommitmentKey { * @return Commitment */ Commitment commit_structured(PolynomialSpan polynomial, - const std::vector>& active_ranges) + const std::vector>& active_ranges, + size_t final_active_wire_idx = 0) { - BB_OP_COUNT_TIME(); + PROFILE_THIS_NAME("commit_structured"); ASSERT(polynomial.end_index() <= srs->get_monomial_size()); // Percentage of nonzero coefficients beyond which we resort to the conventional commit method constexpr size_t NONZERO_THRESHOLD = 75; + // Compute the number of non-zero coefficients in the polynomial size_t total_num_scalars = 0; - for (const auto& range : active_ranges) { - total_num_scalars += range.second - range.first; + for (const auto& [first, second] : active_ranges) { + total_num_scalars += second - first; } // Compute "active" percentage of polynomial; resort to standard commit if appropriate - size_t percentage_nonzero = total_num_scalars * 100 / polynomial.size(); + size_t polynomial_size = final_active_wire_idx != 0 ? final_active_wire_idx : polynomial.size(); + size_t percentage_nonzero = total_num_scalars * 100 / polynomial_size; if (percentage_nonzero > NONZERO_THRESHOLD) { return commit(polynomial); } @@ -259,9 +262,10 @@ template class CommitmentKey { * @return Commitment */ Commitment commit_structured_with_nonzero_complement(PolynomialSpan polynomial, - const std::vector>& active_ranges) + const std::vector>& active_ranges, + size_t final_active_wire_idx = 0) { - BB_OP_COUNT_TIME(); + PROFILE_THIS_NAME("commit_structured_with_nonzero_complement"); ASSERT(polynomial.end_index() <= srs->get_monomial_size()); using BatchedAddition = BatchedAffineAddition; @@ -273,20 +277,21 @@ template class CommitmentKey { // Note: the range from the end of the last active range to the end of the polynomial is excluded from the // complement since the polynomial is assumed to be zero there. std::vector> active_ranges_complement; + // Also compute total number of scalars in the constant regions + size_t total_num_complement_scalars = 0; for (size_t i = 0; i < active_ranges.size() - 1; ++i) { const size_t start = active_ranges[i].second; const size_t end = active_ranges[i + 1].first; - active_ranges_complement.emplace_back(start, end); - } - - // Compute the total number of scalars in the constant regions - size_t total_num_complement_scalars = 0; - for (const auto& range : active_ranges_complement) { - total_num_complement_scalars += range.second - range.first; + if (end > start) { + active_ranges_complement.emplace_back(start, end); + total_num_complement_scalars += end - start; + } } + size_t polynomial_size = final_active_wire_idx != 0 ? final_active_wire_idx : polynomial.size(); // Compute percentage of polynomial comprised of constant blocks; resort to standard commit if appropriate - size_t percentage_constant = total_num_complement_scalars * 100 / polynomial.size(); + size_t percentage_constant = total_num_complement_scalars * 100 / polynomial_size; + if (percentage_constant < CONSTANT_THRESHOLD) { return commit(polynomial); } @@ -299,12 +304,11 @@ template class CommitmentKey { // TODO(https://github.com/AztecProtocol/barretenberg/issues/1131): Peak memory usage could be improved by // performing this copy and the subsequent summation as a precomputation prior to constructing the point table. std::vector points; - points.reserve(2 * total_num_complement_scalars); - for (const auto& range : active_ranges_complement) { - const size_t start = 2 * range.first; - const size_t end = 2 * range.second; - for (size_t i = start; i < end; i += 2) { - points.emplace_back(point_table[i]); + + points.reserve(total_num_complement_scalars); + for (const auto& [start, end] : active_ranges_complement) { + for (size_t i = start; i < end; i++) { + points.emplace_back(point_table[2 * i]); } } @@ -313,17 +317,16 @@ template class CommitmentKey { std::vector unique_scalars; std::vector sequence_counts; for (const auto& range : active_ranges_complement) { - if (range.second - range.first > 0) { // only ranges with nonzero length - unique_scalars.emplace_back(polynomial.span[range.first]); - sequence_counts.emplace_back(range.second - range.first); - } + unique_scalars.emplace_back(polynomial.span[range.first]); + sequence_counts.emplace_back(range.second - range.first); } // Reduce each sequence to a single point auto reduced_points = BatchedAddition::add_in_place(points, sequence_counts); // Compute the full commitment as the sum of the "active" region commitment and the constant region contribution - Commitment result = commit_structured(polynomial, active_ranges); + Commitment result = commit_structured(polynomial, active_ranges, final_active_wire_idx); + for (auto [scalar, point] : zip_view(unique_scalars, reduced_points)) { result = result + point * scalar; } diff --git a/barretenberg/cpp/src/barretenberg/ecc/batched_affine_addition/batched_affine_addition.cpp b/barretenberg/cpp/src/barretenberg/ecc/batched_affine_addition/batched_affine_addition.cpp index 447e6039c55..058bce37738 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/batched_affine_addition/batched_affine_addition.cpp +++ b/barretenberg/cpp/src/barretenberg/ecc/batched_affine_addition/batched_affine_addition.cpp @@ -10,6 +10,7 @@ template std::vector::G1> BatchedAffineAddition::add_in_place( const std::span& points, const std::vector& sequence_counts) { + PROFILE_THIS_NAME("BatchedAffineAddition::add_in_place"); // Instantiate scratch space for point addition denominators and their calculation std::vector scratch_space_vector(points.size()); std::span scratch_space(scratch_space_vector); diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_impl.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_impl.hpp index 0ed0c481dec..5a747150eb1 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_impl.hpp @@ -404,9 +404,10 @@ template void field::batch_invert(field* coeffs, const size_t n) no batch_invert(std::span{ coeffs, n }); } +// TODO(https://github.com/AztecProtocol/barretenberg/issues/1166) template void field::batch_invert(std::span coeffs) noexcept { - BB_OP_COUNT_TRACK_NAME("fr::batch_invert"); + PROFILE_THIS_NAME("fr::batch_invert"); const size_t n = coeffs.size(); auto temporaries_ptr = std::static_pointer_cast(get_mem_slab(n * sizeof(field))); diff --git a/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp b/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp index b9230e4675a..e6fe7ef2aa3 100644 --- a/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/flavor/flavor.hpp @@ -123,7 +123,7 @@ template class ProvingKey_ { // folded element by element. std::vector public_inputs; - // Ranges of the form [start, end) over which the execution trace is "active" + // Ranges of the form [start, end) where witnesses have non-zero values (hence the execution trace is "active") std::vector> active_block_ranges; ProvingKey_() = default; diff --git a/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/execution_trace_usage_tracker.hpp b/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/execution_trace_usage_tracker.hpp index 683a20a3d63..91837b267dd 100644 --- a/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/execution_trace_usage_tracker.hpp +++ b/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/execution_trace_usage_tracker.hpp @@ -22,6 +22,7 @@ struct ExecutionTraceUsageTracker { MegaTraceFixedBlockSizes fixed_sizes; // fixed size of each block prescribed by structuring // Store active ranges based on the most current accumulator and those based on all but the most recently // accumulated circuit. The former is needed for the combiner calculation and the latter for the perturbator. + // The ranges cover all areas in the trace where relations have nontrivial values. std::vector active_ranges; std::vector previous_active_ranges; diff --git a/barretenberg/cpp/src/barretenberg/plonk_honk_shared/library/grand_product_library.hpp b/barretenberg/cpp/src/barretenberg/plonk_honk_shared/library/grand_product_library.hpp index 58b1d35630a..ee7eeb3c437 100644 --- a/barretenberg/cpp/src/barretenberg/plonk_honk_shared/library/grand_product_library.hpp +++ b/barretenberg/cpp/src/barretenberg/plonk_honk_shared/library/grand_product_library.hpp @@ -56,8 +56,11 @@ namespace bb { template void compute_grand_product(typename Flavor::ProverPolynomials& full_polynomials, bb::RelationParameters& relation_parameters, - size_t size_override = 0) + size_t size_override = 0, + std::vector> active_block_ranges = {}) { + PROFILE_THIS_NAME("compute_grand_product"); + using FF = typename Flavor::FF; using Polynomial = typename Flavor::Polynomial; using Accumulator = std::tuple_element_t<0, typename GrandProdRelation::SumcheckArrayOfValuesOverSubrelations>; @@ -84,22 +87,34 @@ void compute_grand_product(typename Flavor::ProverPolynomials& full_polynomials, Polynomial numerator{ domain_size, domain_size }; Polynomial denominator{ domain_size, domain_size }; + auto check_is_active = [&](size_t idx) { + if (active_block_ranges.empty()) { + return true; + } + return std::any_of(active_block_ranges.begin(), active_block_ranges.end(), [idx](const auto& range) { + return idx >= range.first && idx < range.second; + }); + }; + // Step (1) // Populate `numerator` and `denominator` with the algebra described by Relation + FF gamma_fourth = relation_parameters.gamma.pow(4); parallel_for(num_threads, [&](size_t thread_idx) { - typename Flavor::AllValues evaluations; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/940): construction of evaluations is equivalent to - // calling get_row which creates full copies. avoid? + typename Flavor::AllValues row; const size_t start = idx_bounds[thread_idx].first; const size_t end = idx_bounds[thread_idx].second; for (size_t i = start; i < end; ++i) { - for (auto [eval, full_poly] : zip_view(evaluations.get_all(), full_polynomials.get_all())) { - eval = full_poly.size() > i ? full_poly[i] : 0; + if (check_is_active(i)) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/940):consider avoiding get_row if possible. + row = full_polynomials.get_row(i); + numerator.at(i) = + GrandProdRelation::template compute_grand_product_numerator(row, relation_parameters); + denominator.at(i) = GrandProdRelation::template compute_grand_product_denominator( + row, relation_parameters); + } else { + numerator.at(i) = gamma_fourth; + denominator.at(i) = gamma_fourth; } - numerator.at(i) = GrandProdRelation::template compute_grand_product_numerator( - evaluations, relation_parameters); - denominator.at(i) = GrandProdRelation::template compute_grand_product_denominator( - evaluations, relation_parameters); } }); @@ -163,6 +178,7 @@ void compute_grand_product(typename Flavor::ProverPolynomials& full_polynomials, auto& grand_product_polynomial = GrandProdRelation::get_grand_product_polynomial(full_polynomials); // We have a 'virtual' 0 at the start (as this is a to-be-shifted polynomial) ASSERT(grand_product_polynomial.start_index() == 1); + parallel_for(num_threads, [&](size_t thread_idx) { const size_t start = idx_bounds[thread_idx].first; const size_t end = idx_bounds[thread_idx].second; diff --git a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp index 3b00b5bff69..d51062f991f 100644 --- a/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/databus_lookup_relation.hpp @@ -231,6 +231,7 @@ template class DatabusLookupRelationImpl { auto& relation_parameters, const size_t circuit_size) { + PROFILE_THIS_NAME("Databus::compute_logderivative_inverse"); auto& inverse_polynomial = BusData::inverses(polynomials); size_t min_iterations_per_thread = 1 << 6; // min number of iterations for which we'll spin up a unique thread diff --git a/barretenberg/cpp/src/barretenberg/relations/logderiv_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/logderiv_lookup_relation.hpp index 686b751903c..dbd45cc2ed5 100644 --- a/barretenberg/cpp/src/barretenberg/relations/logderiv_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/logderiv_lookup_relation.hpp @@ -156,6 +156,7 @@ template class LogDerivLookupRelationImpl { auto& relation_parameters, const size_t circuit_size) { + PROFILE_THIS_NAME("Lookup::compute_logderivative_inverse"); auto& inverse_polynomial = get_inverse_polynomial(polynomials); size_t min_iterations_per_thread = 1 << 6; // min number of iterations for which we'll spin up a unique thread diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_flavor.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_flavor.hpp index f4129c363a3..24919feb257 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_flavor.hpp @@ -492,6 +492,8 @@ class MegaFlavor { */ void compute_logderivative_inverses(const RelationParameters& relation_parameters) { + PROFILE_THIS_NAME("compute_logderivative_inverses"); + // Compute inverses for conventional lookups LogDerivLookupRelation::compute_logderivative_inverse( this->polynomials, relation_parameters, this->circuit_size); @@ -525,7 +527,7 @@ class MegaFlavor { // Compute permutation grand product polynomial compute_grand_product>( - this->polynomials, relation_parameters, size_override); + this->polynomials, relation_parameters, size_override, this->active_block_ranges); } uint64_t estimate_memory() diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_flavor.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_flavor.hpp index 6ff45fb338d..f34d0b48733 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_flavor.hpp @@ -328,7 +328,7 @@ class UltraFlavor { [[nodiscard]] size_t get_polynomial_size() const { return q_c.size(); } [[nodiscard]] AllValues get_row(const size_t row_idx) const { - PROFILE_THIS(); + PROFILE_THIS_NAME("UltraFlavor::get_row"); AllValues result; for (auto [result_field, polynomial] : zip_view(result.get_all(), get_all())) { result_field = polynomial[row_idx]; diff --git a/barretenberg/cpp/src/barretenberg/trace_to_polynomials/trace_to_polynomials.cpp b/barretenberg/cpp/src/barretenberg/trace_to_polynomials/trace_to_polynomials.cpp index 549d42564fd..39eccd6ef01 100644 --- a/barretenberg/cpp/src/barretenberg/trace_to_polynomials/trace_to_polynomials.cpp +++ b/barretenberg/cpp/src/barretenberg/trace_to_polynomials/trace_to_polynomials.cpp @@ -150,6 +150,7 @@ typename TraceToPolynomials::TraceData TraceToPolynomials::const // otherwise, the next block starts immediately following the previous one offset += block.get_fixed_size(is_structured); } + return trace_data; } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp index c19a43f8f98..7377c7b31c7 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp @@ -235,6 +235,7 @@ template void OinkProver::execute_grand_product_c { PROFILE_THIS_NAME("OinkProver::execute_grand_product_computation_round"); // Compute the permutation grand product polynomial + proving_key->proving_key.compute_grand_product_polynomial(proving_key->relation_parameters, proving_key->final_active_wire_idx + 1); @@ -243,7 +244,9 @@ template void OinkProver::execute_grand_product_c if (proving_key->get_is_structured()) { witness_commitments.z_perm = proving_key->proving_key.commitment_key->commit_structured_with_nonzero_complement( - proving_key->proving_key.polynomials.z_perm, proving_key->proving_key.active_block_ranges); + proving_key->proving_key.polynomials.z_perm, + proving_key->proving_key.active_block_ranges, + proving_key->final_active_wire_idx + 1); } else { witness_commitments.z_perm = proving_key->proving_key.commitment_key->commit(proving_key->proving_key.polynomials.z_perm); diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.hpp b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.hpp index 8ab9fed7aa1..e6db742510b 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.hpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.hpp @@ -19,6 +19,7 @@ // clang-format on #include +#include "barretenberg/plonk_honk_shared/execution_trace/execution_trace_usage_tracker.hpp" #include "barretenberg/ultra_honk/decider_proving_key.hpp" namespace bb { @@ -40,16 +41,20 @@ template class OinkProver { std::shared_ptr proving_key; std::shared_ptr transcript; std::string domain_separator; + ExecutionTraceUsageTracker trace_usage_tracker; + typename Flavor::WitnessCommitments witness_commitments; typename Flavor::CommitmentLabels commitment_labels; using RelationSeparator = typename Flavor::RelationSeparator; OinkProver(std::shared_ptr proving_key, const std::shared_ptr& transcript = std::make_shared(), - std::string domain_separator = "") + std::string domain_separator = "", + const ExecutionTraceUsageTracker& trace_usage_tracker = ExecutionTraceUsageTracker{}) : proving_key(proving_key) , transcript(transcript) , domain_separator(std::move(domain_separator)) + , trace_usage_tracker(trace_usage_tracker) {} void prove();