Skip to content

Commit

Permalink
feat: Use ranges instead of a vector for input witness (#3314)
Browse files Browse the repository at this point in the history
  • Loading branch information
guipublic authored Nov 3, 2023
1 parent 466cb07 commit b12b7ec
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 33 deletions.
25 changes: 19 additions & 6 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
//! This module heavily borrows from Cranelift
#![allow(dead_code)]

use std::collections::BTreeSet;
use std::{
collections::{BTreeMap, BTreeSet},
ops::Range,
};

use crate::errors::{RuntimeError, SsaReport};
use acvm::acir::{
Expand Down Expand Up @@ -87,14 +90,12 @@ pub fn create_circuit(
..
} = generated_acir;

let abi = gen_abi(context, func_sig, &input_witnesses, return_witnesses.clone());
let abi = gen_abi(context, func_sig, input_witnesses, return_witnesses.clone());
let public_abi = abi.clone().public_abi();

let public_parameters =
PublicInputs(public_abi.param_witnesses.values().flatten().copied().collect());
let public_parameters = PublicInputs(tree_to_set(&public_abi.param_witnesses));

let all_parameters: BTreeSet<Witness> =
abi.param_witnesses.values().flatten().copied().collect();
let all_parameters: BTreeSet<Witness> = tree_to_set(&abi.param_witnesses);
let private_parameters = all_parameters.difference(&public_parameters.0).copied().collect();

let return_values = PublicInputs(return_witnesses.into_iter().collect());
Expand Down Expand Up @@ -162,3 +163,15 @@ impl SsaBuilder {
self
}
}

// Flatten the witnesses in the map into a BTreeSet
fn tree_to_set(input: &BTreeMap<String, Vec<Range<Witness>>>) -> BTreeSet<Witness> {
let mut result = BTreeSet::new();
for range in input.values().flatten() {
for i in range.start.witness_index()..range.end.witness_index() {
result.insert(Witness(i));
}
}

result
}
30 changes: 24 additions & 6 deletions compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use noirc_frontend::{
},
node_interner::NodeInterner,
};
use std::ops::Range;

/// Attempts to retrieve the name of this parameter. Returns None
/// if this parameter is a tuple or struct pattern.
Expand Down Expand Up @@ -38,7 +39,7 @@ pub fn into_abi_params(context: &Context, params: Vec<Param>) -> Vec<AbiParamete
pub(crate) fn gen_abi(
context: &Context,
func_sig: FunctionSignature,
input_witnesses: &[Witness],
input_witnesses: Vec<Range<Witness>>,
return_witnesses: Vec<Witness>,
) -> Abi {
let (parameters, return_type) = func_sig;
Expand All @@ -52,16 +53,33 @@ pub(crate) fn gen_abi(
// parameter's constituent values live.
fn param_witnesses_from_abi_param(
abi_params: &Vec<AbiParameter>,
input_witnesses: &[Witness],
) -> BTreeMap<String, Vec<Witness>> {
input_witnesses: Vec<Range<Witness>>,
) -> BTreeMap<String, Vec<Range<Witness>>> {
let mut idx = 0_usize;
if input_witnesses.is_empty() {
return BTreeMap::new();
}
let mut processed_range = input_witnesses[idx].start.witness_index();

btree_map(abi_params, |param| {
let num_field_elements_needed = param.typ.field_count();
let mut wit = Vec::new();
for _ in 0..num_field_elements_needed {
wit.push(input_witnesses[idx]);
idx += 1;
let mut processed_fields = 0;
while processed_fields < num_field_elements_needed {
let end = input_witnesses[idx].end.witness_index();
if num_field_elements_needed <= end - processed_range {
wit.push(
Witness(processed_range)..Witness(processed_range + num_field_elements_needed),
);
processed_range += num_field_elements_needed;
processed_fields += num_field_elements_needed;
} else {
// consume the current range
wit.push(Witness(processed_range)..input_witnesses[idx].end);
processed_fields += end - processed_range;
idx += 1;
processed_range = input_witnesses[idx].start.witness_index();
}
}
(param.name.clone(), wit)
})
Expand Down
26 changes: 24 additions & 2 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use acvm::{BlackBoxFunctionSolver, BlackBoxResolutionError};
use fxhash::FxHashMap as HashMap;
use iter_extended::{try_vecmap, vecmap};
use num_bigint::BigUint;
use std::ops::RangeInclusive;
use std::{borrow::Cow, hash::Hash};

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -1176,8 +1177,29 @@ impl AcirContext {
}

/// Terminates the context and takes the resulting `GeneratedAcir`
pub(crate) fn finish(mut self, inputs: Vec<u32>, warnings: Vec<SsaReport>) -> GeneratedAcir {
self.acir_ir.input_witnesses = vecmap(inputs, Witness);
pub(crate) fn finish(
mut self,
inputs: Vec<RangeInclusive<u32>>,
warnings: Vec<SsaReport>,
) -> GeneratedAcir {
let mut current_range = 0..0;
for range in inputs {
if current_range.end == *range.start() {
current_range.end = range.end() + 1;
} else {
if current_range.end != 0 {
self.acir_ir
.input_witnesses
.push(Witness(current_range.start)..Witness(current_range.end));
}
current_range = *range.start()..range.end() + 1;
}
}
if current_range.end != 0 {
self.acir_ir
.input_witnesses
.push(Witness(current_range.start)..Witness(current_range.end));
}
self.acir_ir.warnings = warnings;
self.acir_ir
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use acvm::{
};
use iter_extended::vecmap;
use num_bigint::BigUint;
use std::ops::Range;

#[derive(Debug, Default)]
/// The output of the Acir-gen pass
Expand All @@ -42,7 +43,7 @@ pub(crate) struct GeneratedAcir {
pub(crate) return_witnesses: Vec<Witness>,

/// All witness indices which are inputs to the main function
pub(crate) input_witnesses: Vec<Witness>,
pub(crate) input_witnesses: Vec<Range<Witness>>,

/// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it
pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
Expand Down
7 changes: 4 additions & 3 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::brillig::brillig_ir::BrilligContext;
use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
pub(crate) use acir_ir::generated_acir::GeneratedAcir;

use acvm::{
acir::{circuit::opcodes::BlockId, native_types::Expression},
FieldElement,
Expand Down Expand Up @@ -203,7 +204,7 @@ impl Context {

let warnings = self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?;

Ok(self.acir_context.finish(input_witness.collect(), warnings))
Ok(self.acir_context.finish(vec![input_witness], warnings))
}

fn convert_brillig_main(
Expand Down Expand Up @@ -239,8 +240,8 @@ impl Context {
for acir_var in output_vars {
self.acir_context.return_var(acir_var)?;
}

Ok(self.acir_context.finish(witness_inputs, Vec::new()))
let witnesses = vecmap(witness_inputs, |input| RangeInclusive::new(input, input));
Ok(self.acir_context.finish(witnesses, Vec::new()))
}

/// Adds and binds `AcirVar`s for each numeric block parameter or block parameter array element.
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"hash":13834844072603749544,"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"private"},{"name":"y","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"public"}],"param_witnesses":{"x":[1],"y":[2]},"return_type":{"kind":"integer","sign":"unsigned","width":64},"return_witnesses":[12]},"bytecode":"H4sIAAAAAAAA/+1WUW6DMAx1QksZoGr72jUcAiX8VbvJ0Oj9j7ChJpKbtXw0NpvUWkImUXixn53w3gDgHc6mfh7t/ZGMtR9TU96HeYuHtp36ZjLWfGIzjK7DthsPzjjTue6rcdZOrnX9MA49Dqa1kzl1gz3h2bL7sTDCMhmJbylmTDOT8WEhjXfjH/DcB8u8zwVygWifmL/9lTnWzSWKsxHA3QJf00vlveWvERJIUU4x0eb86aEJppljVox9oO+Py8QTV1Jnw6a85t7vSL8pwvN89j7gd88o8q79Gr2wRt3AeSFz4XvRSyokl5MAtSfgGO2ZCewdsDibLRVrDzIXTMxfqiLIGXPeMdY1gb/Fg8+tznJY50eSGmfB2DNrqciCD+tCRc4X5FNFJmIWnkhu3BL+t4qc8y75aySqIkvGOP9CRWKaGQ0ydUrsgUUVWXlfw4OpyAouVWQN66pITDPDqSJfQaZxuVVkxZhzzVgLTv5uHbDwXhN+vwGywklHPBQAAA=="}
{"hash":13834844072603749544,"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"private"},{"name":"y","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"public"}],"param_witnesses":{"x":[{ "start": 1, "end": 2 }],"y":[{ "start": 2, "end": 3 }]},"return_type":{"kind":"integer","sign":"unsigned","width":64},"return_witnesses":[12]},"bytecode":"H4sIAAAAAAAA/+1WUW6DMAx1QksZoGr72jUcAiX8VbvJ0Oj9j7ChJpKbtXw0NpvUWkImUXixn53w3gDgHc6mfh7t/ZGMtR9TU96HeYuHtp36ZjLWfGIzjK7DthsPzjjTue6rcdZOrnX9MA49Dqa1kzl1gz3h2bL7sTDCMhmJbylmTDOT8WEhjXfjH/DcB8u8zwVygWifmL/9lTnWzSWKsxHA3QJf00vlveWvERJIUU4x0eb86aEJppljVox9oO+Py8QTV1Jnw6a85t7vSL8pwvN89j7gd88o8q79Gr2wRt3AeSFz4XvRSyokl5MAtSfgGO2ZCewdsDibLRVrDzIXTMxfqiLIGXPeMdY1gb/Fg8+tznJY50eSGmfB2DNrqciCD+tCRc4X5FNFJmIWnkhu3BL+t4qc8y75aySqIkvGOP9CRWKaGQ0ydUrsgUUVWXlfw4OpyAouVWQN66pITDPDqSJfQaZxuVVkxZhzzVgLTv5uHbDwXhN+vwGywklHPBQAAA=="}
27 changes: 19 additions & 8 deletions tooling/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use std::{collections::BTreeMap, str};

use acvm::{
acir::native_types::{Witness, WitnessMap},
FieldElement,
Expand All @@ -16,6 +14,8 @@ use noirc_frontend::{
hir::Context, Signedness, StructType, Type, TypeBinding, TypeVariableKind, Visibility,
};
use serde::{Deserialize, Serialize};
use std::ops::Range;
use std::{collections::BTreeMap, str};
// This is the ABI used to bridge the different TOML formats for the initial
// witness, the partial witness generator and the interpreter.
//
Expand Down Expand Up @@ -218,7 +218,7 @@ pub struct Abi {
pub parameters: Vec<AbiParameter>,
/// A map from the ABI's parameters to the indices they are written to in the [`WitnessMap`].
/// This defines how to convert between the [`InputMap`] and [`WitnessMap`].
pub param_witnesses: BTreeMap<String, Vec<Witness>>,
pub param_witnesses: BTreeMap<String, Vec<Range<Witness>>>,
pub return_type: Option<AbiType>,
pub return_witnesses: Vec<Witness>,
}
Expand Down Expand Up @@ -315,13 +315,14 @@ impl Abi {
let mut witness_map: BTreeMap<Witness, FieldElement> = encoded_input_map
.iter()
.flat_map(|(param_name, encoded_param_fields)| {
let param_witness_indices = &self.param_witnesses[param_name];
let param_witness_indices = range_to_vec(&self.param_witnesses[param_name]);
param_witness_indices
.iter()
.zip(encoded_param_fields.iter())
.map(|(&witness, &field_element)| (witness, field_element))
.collect::<Vec<_>>()
})
.collect();
.collect::<BTreeMap<Witness, FieldElement>>();

// When encoding public inputs to be passed to the verifier, the user can must provide a return value
// to be inserted into the witness map. This is not needed when generating a witness when proving the circuit.
Expand Down Expand Up @@ -398,7 +399,7 @@ impl Abi {
let public_inputs_map =
try_btree_map(self.parameters.clone(), |AbiParameter { name, typ, .. }| {
let param_witness_values =
try_vecmap(self.param_witnesses[&name].clone(), |witness_index| {
try_vecmap(range_to_vec(&self.param_witnesses[&name]), |witness_index| {
witness_map
.get(&witness_index)
.ok_or_else(|| AbiError::MissingParamWitnessValue {
Expand Down Expand Up @@ -529,6 +530,16 @@ impl ContractEvent {
}
}

fn range_to_vec(ranges: &[Range<Witness>]) -> Vec<Witness> {
let mut result = Vec::new();
for range in ranges {
for witness in range.start.witness_index()..range.end.witness_index() {
result.push(witness.into());
}
}
result
}

#[cfg(test)]
mod test {
use std::collections::BTreeMap;
Expand All @@ -554,8 +565,8 @@ mod test {
],
// Note that the return value shares a witness with `thing2`
param_witnesses: BTreeMap::from([
("thing1".to_string(), vec![Witness(1), Witness(2)]),
("thing2".to_string(), vec![Witness(3)]),
("thing1".to_string(), vec![(Witness(1)..Witness(3))]),
("thing2".to_string(), vec![(Witness(3)..Witness(4))]),
]),
return_type: Some(AbiType::Field),
return_witnesses: vec![Witness(3)],
Expand Down
4 changes: 2 additions & 2 deletions tooling/noirc_abi_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export type AbiType =
{ kind: "array", length: number, type: AbiType } |
{ kind: "tuple", fields: AbiType[] } |
{ kind: "struct", path: string, fields: [string, AbiType][] };
export type AbiParameter = {
name: string,
type: AbiType,
Expand All @@ -66,7 +66,7 @@ export type AbiParameter = {
export type Abi = {
parameters: AbiParameter[],
param_witnesses: Record<string, number[]>,
param_witnesses: Record<string, {start: number, end: number}[]>,
return_type: AbiType | null,
return_witnesses: number[],
}
Expand Down
2 changes: 1 addition & 1 deletion tooling/noirc_abi_wasm/test/shared/abi_encode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const abi: Abi = {
visibility: 'private',
},
],
param_witnesses: { foo: [1], bar: [2, 3] },
param_witnesses: { foo: [{ start: 1, end: 2 }], bar: [{ start: 2, end: 4 }] },
return_type: null,
return_witnesses: [],
};
Expand Down
2 changes: 1 addition & 1 deletion tooling/noirc_abi_wasm/test/shared/array_as_field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const abi: Abi = {
visibility: 'private',
},
],
param_witnesses: { foo: [1, 2] },
param_witnesses: { foo: [{ start: 1, end: 3 }] },
return_type: null,
return_witnesses: [],
};
Expand Down
2 changes: 1 addition & 1 deletion tooling/noirc_abi_wasm/test/shared/field_as_array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const abi: Abi = {
visibility: 'private',
},
],
param_witnesses: { foo: [1, 2] },
param_witnesses: { foo: [{ start: 1, end: 3 }] },
return_type: null,
return_witnesses: [],
};
Expand Down
2 changes: 1 addition & 1 deletion tooling/noirc_abi_wasm/test/shared/uint_overflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const abi: Abi = {
visibility: 'private',
},
],
param_witnesses: { foo: [1] },
param_witnesses: { foo: [{ start: 1, end: 2 }] },
return_type: null,
return_witnesses: [],
};
Expand Down

0 comments on commit b12b7ec

Please sign in to comment.