Skip to content

Commit

Permalink
switch order of execution
Browse files Browse the repository at this point in the history
  • Loading branch information
vezenovm committed Jan 23, 2024
1 parent 7fdf5be commit f131b96
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 110 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 15 additions & 3 deletions acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ pub struct ACVM<'a, B: BlackBoxFunctionSolver> {
witness_map: WitnessMap,

brillig_solver: Option<BrilligSolver<'a, B>>,

current_assert_message: Option<String>,
}

impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
Expand All @@ -153,6 +155,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
instruction_pointer: 0,
witness_map: initial_witness,
brillig_solver: None,
current_assert_message: None,
}
}

Expand Down Expand Up @@ -203,7 +206,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
/// Sets the VM status to [ACVMStatus::Failure] using the provided `error`.
/// Returns the new status.
fn fail(&mut self, error: OpcodeResolutionError) -> ACVMStatus {
self.instruction_pointer += 1;
// self.instruction_pointer += 1;
self.status(ACVMStatus::Failure(error))
}

Expand Down Expand Up @@ -235,7 +238,12 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
ACVMForeignCallResult::BrilligOutput(foreign_call_result) => {
brillig_solver.resolve_pending_foreign_call(foreign_call_result);
}
ACVMForeignCallResult::ResolvedAssertMessage(_) => {
ACVMForeignCallResult::ResolvedAssertMessage(assert_message) => {
if assert_message.is_empty() {
self.current_assert_message = None;
} else {
self.current_assert_message = Some(assert_message);
}
brillig_solver.resolve_pending_foreign_call(ForeignCallResult::default());
}
}
Expand All @@ -259,7 +267,6 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {

pub fn solve_opcode(&mut self) -> ACVMStatus {
let opcode = &self.opcodes[self.instruction_pointer];

let resolution = match opcode {
Opcode::AssertZero(expr) => ExpressionSolver::solve(&mut self.witness_map, expr),
Opcode::BlackBoxFuncCall(bb_func) => {
Expand Down Expand Up @@ -296,6 +303,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
}
}
Err(mut error) => {
dbg!("got error");
match &mut error {
// If we have an index out of bounds or an unsatisfied constraint, the opcode label will be unresolved
// because the solvers do not have knowledge of this information.
Expand Down Expand Up @@ -384,6 +392,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
self.brillig_solver = Some(solver);
self.solve_opcode()
}

pub fn get_assert_message(&self) -> &Option<String> {
&self.current_assert_message
}
}

// Returns the concrete value for a particular witness
Expand Down
3 changes: 0 additions & 3 deletions acvm-repo/brillig_vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,6 @@ impl<'a, B: BlackBoxFunctionSolver> VM<'a, B> {
/// Indicating that the VM encountered a `Trap` Opcode
/// or an invalid state.
fn fail(&mut self, message: String) -> VMStatus {
dbg!(self.program_counter);
dbg!(&self.bytecode[self.program_counter]);
dbg!(&self.bytecode[self.program_counter - 1]);
let mut error_stack: Vec<_> =
self.call_stack.iter().map(|value| value.to_usize()).collect();
error_stack.push(self.program_counter);
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ pub(crate) fn optimize_into_acir(
.run_pass(Ssa::mem2reg, "After Mem2Reg:")
.run_pass(Ssa::fold_constants, "After Constant Folding:")
.run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
.run_pass(Ssa::bubble_up_constrains, "After Constraint Bubbling:")
.finish();

let brillig = ssa.to_brillig(print_brillig_trace);
Expand Down
34 changes: 20 additions & 14 deletions noirc_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use noirc_frontend::macros_api::SortedModule;
use noirc_frontend::macros_api::{CrateId, FileId};
use noirc_frontend::macros_api::{
Expression, ExpressionKind, HirContext, Ident, Path, PathKind, Span, Statement, StatementKind,
Literal,
};
use noirc_frontend::macros_api::{MacroError, MacroProcessor};

Expand Down Expand Up @@ -42,7 +43,6 @@ fn transform(mut ast: SortedModule) -> Result<SortedModule, (MacroError, FileId)

for func in ast.functions.iter_mut() {
let mut calls_to_insert = Vec::new();
dbg!(func.def.body.0.len());
for (i, stmt) in func.def.body.0.iter().enumerate() {
let Statement { kind, span } = stmt;
if let StatementKind::Constrain(constrain_stmt) = kind {
Expand All @@ -62,30 +62,36 @@ fn transform(mut ast: SortedModule) -> Result<SortedModule, (MacroError, FileId)
vec![assert_msg_expr.clone()],
*span,
);
dbg!(i);
dbg!(i + calls_to_insert.len());
calls_to_insert.push((i + calls_to_insert.len(), call_expr, *span));
} else {
let kind = ExpressionKind::string("".to_owned());
let arg = Expression { kind, span: Span::default() };
let call_expr = Expression::call(
Expression {
kind: ExpressionKind::Variable(Path {
segments: vec![
Ident::from("std"),
Ident::from("resolve_assert_message"),
],
kind: PathKind::Dep,
span: Span::default(),
}),
span: Span::default(),
},
vec![arg],
*span,
);
calls_to_insert.push((i + calls_to_insert.len(), call_expr, *span));
}
}
}
dbg!(func.name());
dbg!(func.def.body.0.len());
if func.name() == "conditional" {
dbg!(func.def.body.0.clone());
}

for (i, call_expr, span) in calls_to_insert {
dbg!(i);
func.def
.body
.0
.insert(i, Statement { kind: StatementKind::Expression(call_expr), span });
}
if func.name() == "conditional" {
dbg!(func.def.body.0.clone());
}
dbg!(func.def.body.0.len());
// dbg!(func.def.body.0.clone());
}
Ok(ast)
}
4 changes: 3 additions & 1 deletion test_programs/execution_success/brillig_assert/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ unconstrained fn conditional(x: bool) -> Field {
// TODO: this is not accurately printing the assert message
// got `Failed to solve brillig function, reason: explicit trap hit in brillig`
// expected `x is false`
assert_eq(x, false, "x is false");
assert_eq(x, true, "x is blah blah");
let z = x as u8 + 20;
assert_eq(z, 25);
1
}
3 changes: 0 additions & 3 deletions test_programs/execution_success/debug_logs/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ fn main(x: Field, y: pub Field) {
// A `fmtstr` lets you easily perform string interpolation.
let fmt_str: fmtstr<14, (Field, Field)> = f"i: {x}, j: {y}";

// TODO
// assert(x == y, fmt_str);

let fmt_str = string_identity(fmt_str);
std::println(fmt_str);

Expand Down
130 changes: 46 additions & 84 deletions tooling/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use acvm::acir::circuit::{OpcodeLocation, Opcode};
use acvm::pwg::{ACVMStatus, ErrorLocation, OpcodeResolutionError, ACVM};
use acvm::acir::circuit::OpcodeLocation;
use acvm::pwg::{ACVMForeignCallResult, ACVMStatus, ErrorLocation, OpcodeResolutionError, ACVM};
use acvm::BlackBoxFunctionSolver;
use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap};

use crate::errors::ExecutionError;
use crate::NargoError;

use super::foreign_calls::{ForeignCallExecutor, ForeignCall};
use super::foreign_calls::ForeignCallExecutor;

#[tracing::instrument(level = "trace", skip_all)]
pub fn execute_circuit<B: BlackBoxFunctionSolver, F: ForeignCallExecutor>(
Expand All @@ -17,70 +17,39 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver, F: ForeignCallExecutor>(
) -> Result<WitnessMap, NargoError> {
let mut acvm = ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness);

let mut err: Option<OpcodeResolutionError> = None;
loop {
if let Some(error) = &err {
// If there are two assertions in a row and the second one is false we could hit
// a failure status that will resolve a comptime assert message rather than a runtime assert
// message as we are expecting.
// If there is a Brillig assertion we are just going to process the next Brillig func rather than
//
dbg!("got err");
// dbg!(&acvm.opcodes()[acvm.instruction_pointer()]);
// dbg!(&acvm.opcodes()[acvm.instruction_pointer() - 1]);
// dbg!(&acvm.opcodes()[acvm.instruction_pointer() - 2]);
let solver_status = acvm.solve();

let call_stack = resolve_call_stack(error);

// Consrtuct error
match call_stack.clone() {
Some((call_stack, is_brillig_fail)) => {
if is_brillig_fail {
dbg!("got brillig fail");
let x = acvm.step_into_brillig_opcode();
}
}
None => {
}
match solver_status {
ACVMStatus::Solved => break,
ACVMStatus::InProgress => {
unreachable!("Execution should not stop while in `InProgress` state.")
}
ACVMStatus::Failure(error) => {
let call_stack = match &error {
OpcodeResolutionError::UnsatisfiedConstrain {
opcode_location: ErrorLocation::Resolved(opcode_location),
} => Some(vec![*opcode_location]),
OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => {
Some(call_stack.clone())
}
_ => None,
};

let solver_status = if can_process_opcode_after_failure(&acvm) {
acvm.solve_opcode()
} else {
return Err(resolve_comptime_assert_message(error, circuit));
};
// dbg!(solver_status.clone());
match solver_status {
ACVMStatus::RequiresForeignCall(foreign_call) => {
let foreign_call_result = foreign_call_executor.execute(&foreign_call)?;

let assert_message = foreign_call_result.get_assert_message().expect("Only assert message resolution is supported for execution after an ACVM failure");

return Err(NargoError::ExecutionError(match call_stack {
Some((call_stack, is_brillig_fail)) => {
ExecutionError::AssertionFailed(assert_message, call_stack)
return Err(NargoError::ExecutionError(match call_stack {
Some(call_stack) => {
if let Some(assert_message) = acvm.get_assert_message() {
ExecutionError::AssertionFailed(assert_message.to_owned(), call_stack)
} else {
ExecutionError::SolvingError(error)
}
None => ExecutionError::SolvingError(error.clone()),
}));
}
_ => return Err(resolve_comptime_assert_message(error, circuit)),
}
None => ExecutionError::SolvingError(error),
}));
}
} else {
let solver_status = acvm.solve();

match solver_status {
ACVMStatus::Solved => break,
ACVMStatus::InProgress => {
unreachable!("Execution should not stop while in `InProgress` state.")
}
ACVMStatus::Failure(error) => {
err = Some(error);
}
ACVMStatus::RequiresForeignCall(foreign_call) => {
// dbg!(foreign_call.clone());
let foreign_call_result = foreign_call_executor.execute(&foreign_call)?;
acvm.resolve_pending_foreign_call(foreign_call_result);
}
ACVMStatus::RequiresForeignCall(foreign_call) => {
let foreign_call_result = foreign_call_executor.execute(&foreign_call)?;
acvm.resolve_pending_foreign_call(foreign_call_result);
}
}
}
Expand Down Expand Up @@ -118,26 +87,19 @@ fn resolve_comptime_assert_message(error: &OpcodeResolutionError, circuit: &Circ
})
}

fn can_process_opcode_after_failure<'a, B: BlackBoxFunctionSolver>(acvm: &ACVM<'a, B>) -> bool {
if acvm.instruction_pointer() >= acvm.opcodes().len() {
return false;
}
if let Opcode::Brillig(brillig) = &acvm.opcodes()[acvm.instruction_pointer()] {
// We do not want
match &brillig.bytecode[brillig.bytecode.len() - 2] {
acvm::brillig_vm::brillig::Opcode::ForeignCall { function, .. } => {
ForeignCall::execution_allowed_after_failure(function)
}
_ => false,
}
// if matches!(&brillig.bytecode[brillig.bytecode.len() - 2], acvm::brillig_vm::brillig::Opcode::ForeignCall { function, .. }) {
// // if function
// // if function
// true
// } else {
// false
// }
} else {
false
}
}
// fn can_process_opcode_after_failure<'a, B: BlackBoxFunctionSolver>(acvm: &ACVM<'a, B>) -> bool {
// if acvm.instruction_pointer() >= acvm.opcodes().len() {
// return false;
// }
// if let Opcode::Brillig(brillig) = &acvm.opcodes()[acvm.instruction_pointer()] {
// // We do not want
// match &brillig.bytecode[brillig.bytecode.len() - 2] {
// acvm::brillig_vm::brillig::Opcode::ForeignCall { function, .. } => {
// ForeignCall::execution_allowed_after_failure(function)
// }
// _ => false,
// }
// } else {
// false
// }
// }

0 comments on commit f131b96

Please sign in to comment.