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(brillig): foreign call/oracle compilation #1600

Merged
merged 42 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
6184f52
remove mac runner
kevaundray Jun 6, 2023
823bd84
chore: generate brillig opcode for simple identity unconstrained func…
guipublic Jun 6, 2023
545d333
feat(brillig): added arithmetic operations on brillig (#1565)
sirasistant Jun 6, 2023
f6810af
make ranges be polymorphic integers
kevaundray Jun 6, 2023
c5e34c5
chore(brillig): Clean up handling of Binary operations (#1571)
kevaundray Jun 6, 2023
06543b5
chore(ssa refactor): Rename Brillig example (#1563)
kevaundray Jun 6, 2023
3c9f106
chore(brillig): added tests for all field binary operations (#1586)
sirasistant Jun 7, 2023
9a9c461
chore(brillig): added tests for brillig integer operations (#1590)
sirasistant Jun 7, 2023
043c3c4
feat: process blocks and jumps when compiling brillig (#1591)
guipublic Jun 7, 2023
e51d3c6
feat: process blocks and jumps when compiling brillig (#1591)
guipublic Jun 7, 2023
1ed956b
feat(brillig): start of oracles/foreign calls
ludamad Jun 7, 2023
2a797a1
fix: broken tests
ludamad Jun 7, 2023
cd39144
feat(brillig): parsing oracles/foreign calls (#1596)
ludamad Jun 7, 2023
ba8ae00
feat: more foreign call work
ludamad Jun 8, 2023
cf393ff
Merge remote-tracking branch 'origin/master' into kw/brillig-main
kevaundray Jun 8, 2023
4644da9
self.data -> self.vars
kevaundray Jun 8, 2023
d50c69f
Merge remote-tracking branch 'origin/master' into kw/brillig-main
kevaundray Jun 8, 2023
398ddf9
chore(brillig): Add handling of the not instruction (#1609)
kevaundray Jun 8, 2023
c5b9579
Merge
ludamad Jun 8, 2023
aa2185c
Merge master
ludamad Jun 8, 2023
3847f0a
test: brillig oracle
ludamad Jun 8, 2023
828c116
Reinstate option
ludamad Jun 8, 2023
1d52f5e
make behavior consistent
kevaundray Jun 8, 2023
2d3ab61
remove closure
kevaundray Jun 8, 2023
801a739
change index_type
kevaundray Jun 8, 2023
2bcfc24
Update crates/noirc_frontend/src/hir/type_check/expr.rs
jfecher Jun 8, 2023
49a151d
Merge remote-tracking branch 'origin/master' into kw/brillig-main
kevaundray Jun 8, 2023
5247a48
Merge remote-tracking branch 'origin/kw/polymorphic-integers-on-for-l…
kevaundray Jun 8, 2023
5995b30
feat(brillig): loops (#1610)
sirasistant Jun 8, 2023
6d83b22
feat: Foreign calls compiling and basic print executed in nargo (#1612)
vezenovm Jun 8, 2023
1a9d33c
chore: resolve immutable array merge differences (#1617)
joss-aztec Jun 9, 2023
2787cc9
chore(ssa refactor): Add more documentation for truncation (#1607)
kevaundray Jun 9, 2023
8713a89
Merge remote-tracking branch 'origin/master' into kw/brillig-main
kevaundray Jun 9, 2023
70f8fe2
Update .github/workflows/test.yml
kevaundray Jun 9, 2023
91defbc
Update .github/workflows/test.yml
kevaundray Jun 9, 2023
4269ac2
resolve merge conflicts
vezenovm Jun 9, 2023
48405c6
remove dbg
vezenovm Jun 9, 2023
f49d69c
merge conflicts
vezenovm Jun 9, 2023
f309bac
change printer for foreign call
vezenovm Jun 9, 2023
efd84b8
Update crates/noirc_evaluator/src/ssa_refactor/ir/function.rs
kevaundray Jun 9, 2023
f5bbb70
Update crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs
vezenovm Jun 9, 2023
d6c7496
Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_…
kevaundray Jun 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ result
*.pk
*.vk
**/Verifier.toml
**/target
39 changes: 36 additions & 3 deletions crates/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use acvm::pwg::{solve, PartialWitnessGeneratorStatus};
use acvm::acir::brillig_vm::ForeignCallResult;
use acvm::acir::circuit::Opcode;
use acvm::pwg::{solve, PartialWitnessGeneratorStatus, UnresolvedBrilligCall};
use acvm::PartialWitnessGenerator;
use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap, pwg::block::Blocks};

Expand All @@ -11,8 +13,39 @@ pub fn execute_circuit(
) -> Result<WitnessMap, NargoError> {
let mut blocks = Blocks::default();
let solver_status = solve(backend, &mut initial_witness, &mut blocks, circuit.opcodes)?;
if matches!(solver_status, PartialWitnessGeneratorStatus::RequiresOracleData { .. }) {
todo!("Add oracle support to nargo execute")

// TODO(#1615): Nargo only supports "oracle_print_impl" functions that print a singular value and nothing else
// expand this in a general logging refactor
if let PartialWitnessGeneratorStatus::RequiresOracleData {
unresolved_brillig_calls,
required_oracle_data,
unsolved_opcodes,
} = solver_status
{
if !required_oracle_data.is_empty() {
unreachable!("oracles are not supported by nargo execute")
}
for unresolved_brillig_call in unresolved_brillig_calls {
let UnresolvedBrilligCall { foreign_call_wait_info, mut brillig } =
unresolved_brillig_call;
let value = foreign_call_wait_info.inputs[0];

// Execute foreign call "oracle_print_impl"
println!("{:?}", value.to_field().to_hex());

// TODO(#1615): "oracle_print_impl" is just an identity func
brillig.foreign_call_results.push(ForeignCallResult { values: vec![value] });

let mut next_opcodes_for_solving = vec![Opcode::Brillig(brillig)];
next_opcodes_for_solving.extend_from_slice(&unsolved_opcodes[..]);

let solver_status =
solve(backend, &mut initial_witness, &mut blocks, next_opcodes_for_solving)?;
if matches!(solver_status, PartialWitnessGeneratorStatus::RequiresOracleData { .. }) {
todo!("Add multiple foreign call support to nargo execute")
// TODO 1557
}
}
}

Ok(initial_witness)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = "10"

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Tests oracle usage in brillig/unconstrained functions
fn main(x: Field) {
// call through a brillig wrapper
oracle_print_wrapper(x);
}


#[oracle(oracle_print_impl)]
unconstrained fn oracle_print(_x : Field) {}

unconstrained fn oracle_print_wrapper(x: Field) {
oracle_print(x);
}


20 changes: 19 additions & 1 deletion crates/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ use crate::ssa_refactor::ir::{
};
use acvm::{
acir::brillig_vm::{
BinaryIntOp, Opcode as BrilligOpcode, RegisterIndex, Value as BrilligValue,
BinaryIntOp, Opcode as BrilligOpcode, RegisterIndex, RegisterValueOrArray,
Value as BrilligValue,
},
FieldElement,
};
use iter_extended::vecmap;
use std::collections::HashMap;

#[derive(Default)]
Expand Down Expand Up @@ -205,6 +207,21 @@ impl BrilligGen {
};
self.push_code(opcode);
}
Instruction::ForeignCall { func, arguments } => {
let result_ids = dfg.instruction_results(instruction_id);

let input_registers =
vecmap(arguments, |value_id| self.convert_ssa_value(*value_id, dfg));
let output_registers =
vecmap(result_ids, |value_id| self.convert_ssa_value(*value_id, dfg));

let opcode = BrilligOpcode::ForeignCall {
function: func.to_owned(),
destination: RegisterValueOrArray::RegisterIndex(output_registers[0]),
input: RegisterValueOrArray::RegisterIndex(input_registers[0]),
};
self.push_code(opcode);
}
_ => todo!("ICE: Instruction not supported {instruction:?}"),
};
}
Expand Down Expand Up @@ -300,6 +317,7 @@ impl BrilligGen {

brillig.convert_ssa_function(func);

brillig.push_code(BrilligOpcode::Stop);
brillig.obj
}

Expand Down
8 changes: 3 additions & 5 deletions crates/noirc_evaluator/src/ssa/ssa_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ impl IrGenerator {
let function_node_id = self.context.get_or_create_opcode_node_id(opcode);
Ok(Value::Node(function_node_id))
}
Definition::Oracle(_, _) => {
unimplemented!("oracles not supported by deprecated SSA")
}
Definition::Oracle(_) => unimplemented!("oracles not supported by deprecated SSA"),
}
}
}
Expand Down Expand Up @@ -499,8 +497,8 @@ impl IrGenerator {
match expr {
Expression::Ident(ident) => self.ssa_gen_identifier(ident),
Expression::Binary(binary) => {
// Note: we disallows structs/tuples in infix expressions.
// The type checker currently disallows this as well but not if they come from generic type
// Note: we disallow structs/tuples in infix expressions.
// The type checker currently disallows this as well but not if they come from a generic type
// We could allow some in the future, e.g. struct == struct
let lhs = self.ssa_gen_expression(&binary.lhs)?.to_node_ids();
let rhs = self.ssa_gen_expression(&binary.rhs)?.to_node_ids();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ impl From<SsaType> for AcirType {

impl<'a> From<&'a SsaType> for AcirType {
fn from(value: &SsaType) -> Self {
dbg!(value.clone());
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
match value {
SsaType::Numeric(numeric_type) => AcirType(*numeric_type),
_ => unreachable!("The type {value} cannot be represented in ACIR"),
Expand Down
24 changes: 19 additions & 5 deletions crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,19 @@ impl Context {
// Generate the brillig code of the function
let code = BrilligArtifact::default().link(&brillig[*id]);
let outputs = self.acir_context.brillig(code, inputs, result_ids.len());

if Self::is_return_type_unit(result_ids, dfg) {
return;
}

for (result, output) in result_ids.iter().zip(outputs) {
let result_acir_type = dfg.type_of_value(*result).into();
self.ssa_values.insert(*result, AcirValue::Var(output, result_acir_type));
}
}
RuntimeType::Oracle(_) => unimplemented!(
"expected an intrinsic/brillig call, but found {func:?}. All Oracle methods should be wrapped in an unconstrained fn"
),
}
}
Value::Intrinsic(intrinsic) => {
Expand Down Expand Up @@ -236,6 +244,7 @@ impl Context {
Instruction::Load { .. } => {
unreachable!("Expected all load instructions to be removed before acir_gen")
}
_ => unreachable!("instruction cannot be converted to ACIR"),
}
}

Expand Down Expand Up @@ -298,11 +307,7 @@ impl Context {
_ => unreachable!("ICE: Program must have a singular return"),
};

// Check if the program returns the `Unit/None` type.
// This type signifies that the program returns nothing.
let is_return_unit_type =
return_values.len() == 1 && dfg.type_of_value(return_values[0]) == Type::Unit;
if is_return_unit_type {
if Self::is_return_type_unit(return_values, dfg) {
return;
}

Expand Down Expand Up @@ -344,6 +349,9 @@ impl Context {
}
Value::Intrinsic(..) => todo!(),
Value::Function(..) => unreachable!("ICE: All functions should have been inlined"),
Value::ForeignFunction(_) => unimplemented!(
"Oracle calls directly in constrained functions are not yet available."
),
Value::Instruction { .. } | Value::Param { .. } => {
unreachable!("ICE: Should have been in cache {value:?}")
}
Expand Down Expand Up @@ -606,6 +614,12 @@ impl Context {
}
}
}

/// Check if the program returns the `Unit/None` type.
/// This type signifies that the program returns nothing.
fn is_return_type_unit(return_values: &[ValueId], dfg: &DataFlowGraph) -> bool {
return_values.len() == 1 && dfg.type_of_value(return_values[0]) == Type::Unit
}
}

#[cfg(test)]
Expand Down
10 changes: 10 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ pub(crate) struct DataFlowGraph {
/// represented by only 1 ValueId within this function.
intrinsics: HashMap<Intrinsic, ValueId>,

/// Contains each foreign function that has been imported into the current function.
/// This map is used to ensure that the ValueId for any given foreign functôn is always
/// represented by only 1 ValueId within this function.
foreign_functions: HashMap<String, ValueId>,

/// Function signatures of external methods
signatures: DenseMap<Signature>,

Expand Down Expand Up @@ -189,6 +194,11 @@ impl DataFlowGraph {
self.values.insert(Value::Function(function))
}

/// Gets or creates a ValueId for the given FunctionId.
pub(crate) fn import_foreign_function(&mut self, function: &str) -> ValueId {
self.values.insert(Value::ForeignFunction(function.to_owned()))
Comment on lines +197 to +199
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment needs to be updated.

We should also have an enumeration for Oracles. It is unclear without it which values may be valid for this string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update comment as part of #1643 refactor. An enumeration would be good for nargo oracle builtins such as println, however, we want to let developers specify their own oracles if they so chose. Need to think on the best way to include these two flows

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, in that case we should likely give them a separate BrilligID like we do for FunctionIds

}

/// Gets or creates a ValueId for the given Intrinsic.
pub(crate) fn import_intrinsic(&mut self, intrinsic: Intrinsic) -> ValueId {
if let Some(existing) = self.intrinsics.get(&intrinsic) {
Expand Down
6 changes: 4 additions & 2 deletions crates/noirc_evaluator/src/ssa_refactor/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ use super::map::Id;
use super::types::Type;
use super::value::ValueId;

#[derive(Clone, Copy, PartialEq, Eq, Debug)]
#[derive(Clone, PartialEq, Eq, Debug)]
pub(crate) enum RuntimeType {
// A noir function, to be compiled in ACIR and executed by ACVM
Acir,
// Unconstrained function, to be compiled to brillig and executed by the Brillig VM
Brillig,
// Oracle function, to be compiled to an external/foreign call
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
Oracle(String),
}
/// A function holds a list of instructions.
/// These instructions are further grouped into Basic blocks
Expand Down Expand Up @@ -59,7 +61,7 @@ impl Function {

/// Runtime type of the function.
pub(crate) fn runtime(&self) -> RuntimeType {
self.runtime
self.runtime.clone()
}

/// Set runtime type of the function.
Expand Down
17 changes: 14 additions & 3 deletions crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ pub(crate) enum Instruction {
/// Performs a function call with a list of its arguments.
Call { func: ValueId, arguments: Vec<ValueId> },

/// Performs an "oracle" - an external function call with a list of its arguments.
/// These are generally unconstrained functions that provide some value lookup.
/// They may result in constraints outside of the context known to Noir.
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
ForeignCall { func: String, arguments: Vec<ValueId> },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need a separate instruction type for this. Instead, we can follow the example used by intrinsic functions and have a separate Value type

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made an issue here: #1643. Good call out, I will refactor this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is already a separate Value type for oracles as well that we can use


/// Allocates a region of memory. Note that this is not concerned with
/// the type of memory, the type of element is determined when loading this memory.
/// This is used for representing mutable variables and references.
Expand Down Expand Up @@ -128,9 +133,10 @@ impl Instruction {
}
Instruction::ArraySet { array, .. } => InstructionResultType::Operand(*array),
Instruction::Constrain(_) | Instruction::Store { .. } => InstructionResultType::None,
Instruction::Load { .. } | Instruction::ArrayGet { .. } | Instruction::Call { .. } => {
InstructionResultType::Unknown
}
Instruction::Load { .. }
| Instruction::ArrayGet { .. }
| Instruction::Call { .. }
| Instruction::ForeignCall { .. } => InstructionResultType::Unknown,
}
}

Expand Down Expand Up @@ -158,6 +164,10 @@ impl Instruction {
max_bit_size: *max_bit_size,
},
Instruction::Constrain(value) => Instruction::Constrain(f(*value)),
Instruction::ForeignCall { func, arguments } => Instruction::ForeignCall {
func: func.to_owned(),
arguments: vecmap(arguments.iter().copied(), f),
},
Instruction::Call { func, arguments } => Instruction::Call {
func: f(*func),
arguments: vecmap(arguments.iter().copied(), f),
Expand Down Expand Up @@ -245,6 +255,7 @@ impl Instruction {
}
Instruction::Truncate { .. } => None,
Instruction::Call { .. } => None,
Instruction::ForeignCall { .. } => None,
Instruction::Allocate { .. } => None,
Instruction::Load { .. } => None,
Instruction::Store { .. } => None,
Expand Down
7 changes: 6 additions & 1 deletion crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ fn value(function: &Function, id: ValueId) -> String {
let elements = vecmap(array, |element| value(function, *element));
format!("[{}]", elements.join(", "))
}
Value::Param { .. } | Value::Instruction { .. } => id.to_string(),
Value::Param { .. } | Value::Instruction { .. } | Value::ForeignFunction(_) => {
id.to_string()
}
}
}

Expand Down Expand Up @@ -148,6 +150,9 @@ pub(crate) fn display_instruction(
Instruction::Call { func, arguments } => {
writeln!(f, "call {}({})", show(*func), value_list(function, arguments))
}
Instruction::ForeignCall { func, arguments } => {
writeln!(f, "foreign call {}({})", func, value_list(function, arguments))
}
Instruction::Allocate => writeln!(f, "allocate"),
Instruction::Load { address } => writeln!(f, "load {}", show(*address)),
Instruction::Store { address, value } => {
Expand Down
6 changes: 6 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ir/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ pub(crate) enum Value {
/// An Intrinsic is a special kind of builtin function that may be handled internally
/// or optimized into a special form.
Intrinsic(Intrinsic),

/// This Value refers to an external function in the IR.
/// ForeignFunction's always have the type Type::Function and have simlar semantics to Function,
/// other than generating different backend operations and being only accessible through Brillig.
ForeignFunction(String),
}

impl Value {
Expand All @@ -62,6 +67,7 @@ impl Value {
Value::Array { element_type, array } => Type::Array(element_type.clone(), array.len()),
Value::Function { .. } => Type::Function,
Value::Intrinsic { .. } => Type::Function,
Value::ForeignFunction { .. } => Type::Function,
}
}
}
5 changes: 4 additions & 1 deletion crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ impl<'function> PerFunctionContext<'function> {
}
Value::Function(function) => self.context.builder.import_function(*function),
Value::Intrinsic(intrinsic) => self.context.builder.import_intrinsic_id(*intrinsic),
Value::ForeignFunction(function) => {
self.context.builder.import_foreign_function(function)
}
Value::Array { array, element_type } => {
let elements = array.iter().map(|value| self.translate_value(*value)).collect();
self.context.builder.array_constant(elements, element_type.clone())
Expand Down Expand Up @@ -327,7 +330,7 @@ impl<'function> PerFunctionContext<'function> {
Instruction::Call { func, arguments } => match self.get_function(*func) {
Some(function) => match ssa.functions[&function].runtime() {
RuntimeType::Acir => self.inline_function(ssa, *id, function, arguments),
RuntimeType::Brillig => {
RuntimeType::Brillig | RuntimeType::Oracle(_) => {
self.context.failed_to_inline_a_call = true;
self.push_instruction(*id);
}
Expand Down
Loading