Skip to content

Commit

Permalink
fix: Don't reuse brillig with slice arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
sirasistant committed Apr 17, 2024
1 parent fd720cc commit aefa5a6
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 13 deletions.
2 changes: 1 addition & 1 deletion l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ library Constants {
uint256 internal constant DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =
0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631;
uint256 internal constant DEPLOYER_CONTRACT_ADDRESS =
0x2d8e7aedc70b65d49e6aa0794d8d12721896c177e87126701f6e60d184358e74;
0x0b98aeb0111208b95d8d71f484f849d7ab44b3e34c545d13736a707ce3cb0839;
uint256 internal constant L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 17;
uint256 internal constant MAX_NOTE_FIELDS_LENGTH = 20;
uint256 internal constant GET_NOTE_ORACLE_RETURN_LENGTH = 23;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ global REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af8166354
// CONTRACT INSTANCE CONSTANTS
// sha224sum 'struct ContractInstanceDeployed'
global DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631;
global DEPLOYER_CONTRACT_ADDRESS = 0x2d8e7aedc70b65d49e6aa0794d8d12721896c177e87126701f6e60d184358e74;
global DEPLOYER_CONTRACT_ADDRESS = 0x0b98aeb0111208b95d8d71f484f849d7ab44b3e34c545d13736a707ce3cb0839;

// NOIR CONSTANTS - constants used only in yarn-packages/noir-contracts
// Some are defined here because Noir doesn't yet support globals referencing other globals yet.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, HashMap};
use crate::ssa::ir::dfg::CallStack;

/// Represents a parameter or a return value of an entry point function.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
pub(crate) enum BrilligParameter {
/// A single address parameter or return value. Holds the bit size of the parameter.
SingleAddr(u32),
Expand Down
30 changes: 20 additions & 10 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,18 @@ struct SharedContext {
/// There can be Brillig functions specified in SSA which do not act as
/// entry points in ACIR (e.g. only called by other Brillig functions)
/// This mapping is necessary to use the correct function pointer for a Brillig call.
brillig_generated_func_pointers: BTreeMap<FunctionId, u32>,
/// This uses the brillig parameters in the map since using slices with different lengths
/// needs to create different brillig entrypoints
brillig_generated_func_pointers: BTreeMap<(FunctionId, Vec<BrilligParameter>), u32>,
}

impl SharedContext {
fn generated_brillig_pointer(&self, func_id: &FunctionId) -> Option<&u32> {
self.brillig_generated_func_pointers.get(func_id)
fn generated_brillig_pointer(
&self,
func_id: FunctionId,
arguments: Vec<BrilligParameter>,
) -> Option<&u32> {
self.brillig_generated_func_pointers.get(&(func_id, arguments))
}

fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig {
Expand All @@ -67,10 +73,11 @@ impl SharedContext {
fn insert_generated_brillig(
&mut self,
func_id: FunctionId,
arguments: Vec<BrilligParameter>,
generated_pointer: u32,
code: GeneratedBrillig,
) {
self.brillig_generated_func_pointers.insert(func_id, generated_pointer);
self.brillig_generated_func_pointers.insert((func_id, arguments), generated_pointer);
self.generated_brillig.push(code);
}

Expand Down Expand Up @@ -356,7 +363,7 @@ impl<'a> Context<'a> {
let outputs: Vec<AcirType> =
vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into());

let code = self.gen_brillig_for(main_func, arguments, brillig)?;
let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?;

// We specifically do not attempt execution of the brillig code being generated as this can result in it being
// replaced with constraints on witnesses to the program outputs.
Expand All @@ -370,7 +377,7 @@ impl<'a> Context<'a> {
// We are guaranteed to have a Brillig function pointer of `0` as main itself is marked as unconstrained
0,
)?;
self.shared_context.insert_generated_brillig(main_func.id(), 0, code);
self.shared_context.insert_generated_brillig(main_func.id(), arguments, 0, code);

let output_vars: Vec<_> = output_values
.iter()
Expand Down Expand Up @@ -657,15 +664,17 @@ impl<'a> Context<'a> {
}
}
let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
let arguments = self.gen_brillig_parameters(arguments, dfg);

let outputs: Vec<AcirType> = vecmap(result_ids, |result_id| {
dfg.type_of_value(*result_id).into()
});

// Check whether we have already generated Brillig for this function
// If we have, re-use the generated code to set-up the Brillig call.
let output_values = if let Some(generated_pointer) =
self.shared_context.generated_brillig_pointer(id)
let output_values = if let Some(generated_pointer) = self
.shared_context
.generated_brillig_pointer(*id, arguments.clone())
{
let code = self
.shared_context
Expand All @@ -680,8 +689,8 @@ impl<'a> Context<'a> {
*generated_pointer,
)?
} else {
let arguments = self.gen_brillig_parameters(arguments, dfg);
let code = self.gen_brillig_for(func, arguments, brillig)?;
let code =
self.gen_brillig_for(func, arguments.clone(), brillig)?;
let generated_pointer =
self.shared_context.new_generated_pointer();
let output_values = self.acir_context.brillig_call(
Expand All @@ -695,6 +704,7 @@ impl<'a> Context<'a> {
)?;
self.shared_context.insert_generated_brillig(
*id,
arguments,
generated_pointer,
code,
);
Expand Down

0 comments on commit aefa5a6

Please sign in to comment.