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

feat: Use ranges instead of a vector for input witness #3314

Merged
merged 15 commits into from
Nov 3, 2023
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