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

chore: Use --show-output flag on execution rather than compilation #2116

Merged
merged 6 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
3 changes: 2 additions & 1 deletion crates/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver + Default>(
_backend: &B,
circuit: Circuit,
initial_witness: WitnessMap,
show_output: bool,
) -> Result<WitnessMap, NargoError> {
let mut acvm = ACVM::new(B::default(), circuit.opcodes, initial_witness);

Expand All @@ -23,7 +24,7 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver + Default>(
}
ACVMStatus::Failure(error) => return Err(error.into()),
ACVMStatus::RequiresForeignCall(foreign_call) => {
let foreign_call_result = ForeignCall::execute(&foreign_call)?;
let foreign_call_result = ForeignCall::execute(&foreign_call, show_output)?;
acvm.resolve_pending_foreign_call(foreign_call_result);
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ impl ForeignCall {

pub(crate) fn execute(
foreign_call: &ForeignCallWaitInfo,
show_output: bool,
) -> Result<ForeignCallResult, ForeignCallError> {
let foreign_call_name = foreign_call.function.as_str();
match Self::lookup(foreign_call_name) {
Some(ForeignCall::Println) => {
Self::execute_println(&foreign_call.inputs)?;
if show_output {
Self::execute_println(&foreign_call.inputs)?;
}
Ok(ForeignCallResult { values: vec![] })
}
Some(ForeignCall::Sequence) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub(crate) fn execute_program<B: Backend>(
debug_data: Option<(DebugInfo, Context)>,
) -> Result<WitnessMap, CliError<B>> {
let initial_witness = abi.encode(inputs_map, None)?;
let solved_witness_err = nargo::ops::execute_circuit(backend, circuit, initial_witness);
let solved_witness_err = nargo::ops::execute_circuit(backend, circuit, initial_witness, true);
match solved_witness_err {
Ok(solved_witness) => Ok(solved_witness),
Err(err) => {
Expand Down
14 changes: 10 additions & 4 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
resolver::resolve_root_manifest,
};

use super::{compile_cmd::optimize_circuit, NargoConfig};
use super::{
compile_cmd::optimize_circuit,
NargoConfig,
};

/// Run the tests for this program
#[derive(Debug, Clone, Args)]
Expand Down Expand Up @@ -96,19 +99,22 @@
show_output: bool,
config: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut program = compile_no_check(context, show_output, config, main)
.map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;
let mut program = compile_no_check(context, config, main).map_err(|err| {
noirc_errors::reporter::report_all(&context.file_manager, &[err], config.deny_warnings);
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
CliError::Generic(format!("Test '{test_name}' failed to compile"))
})?;

// Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`.
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;

// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
match execute_circuit(backend, program.circuit, WitnessMap::new()) {
match execute_circuit(backend, program.circuit, WitnessMap::new(), show_output) {
Ok(_) => Ok(()),
Err(error) => {
let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();
writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).ok();

Check warning on line 117 in crates/nargo_cli/src/cli/test_cmd.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
writeln!(writer, "failed").ok();
writer.reset().ok();
Err(error.into())
Expand Down
16 changes: 13 additions & 3 deletions crates/nargo_cli/tests/test_data/strings/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,26 @@ fn test_prints_strings() {
fn test_prints_array() {
let array = [1, 2, 3, 5, 8];

// TODO: Printing structs currently not supported
// let s = Test { a: 1, b: 2, c: [3, 4] };
// std::println(s);
let s = Test { a: 1, b: 2, c: [3, 4] };
std::println(s);

std::println(array);

let hash = std::hash::pedersen(array);
std::println(hash);
}

fn inner(hex_as_field: Field) {
assert(hex_as_field != 0x41);
let hash = std::hash::pedersen([hex_as_field]);
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
std::println(hash);
}

#[test]
fn test_failed_constraint() {
inner(0x41);
}

struct Test {
a: Field,
b: Field,
Expand Down
8 changes: 3 additions & 5 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
let main_function = context.get_main_function(crate_id)?;

let func_meta = context.def_interner.function_meta(&main_function);

Check warning on line 169 in crates/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
Some(func_meta.into_function_signature(&context.def_interner))
}

Expand All @@ -192,14 +192,14 @@
}
};

let compiled_program = compile_no_check(context, true, options, main)?;
let compiled_program = compile_no_check(context, options, main)?;

if options.print_acir {
println!("Compiled ACIR for main (unoptimized):");
println!("{}", compiled_program.circuit);
}

Ok((compiled_program, warnings))

Check warning on line 202 in crates/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
}

/// Run the frontend to check the crate for errors then compile all contracts if there were none
Expand Down Expand Up @@ -259,7 +259,7 @@
let mut errs = Vec::new();
for function_id in &contract.functions {
let name = context.function_name(function_id).to_owned();
let function = match compile_no_check(context, true, options, *function_id) {
let function = match compile_no_check(context, options, *function_id) {
Ok(function) => function,
Err(err) => {
errs.push(err);
Expand Down Expand Up @@ -296,14 +296,12 @@
#[allow(deprecated)]
pub fn compile_no_check(
context: &Context,
show_output: bool,
options: &CompileOptions,
main_function: FuncId,
) -> Result<CompiledProgram, FileDiagnostic> {
let program = monomorphize(main_function, &context.def_interner);

let (circuit, debug, abi) =
create_circuit(program, options.show_ssa, options.show_brillig, show_output)?;
let (circuit, debug, abi) = create_circuit(program, options.show_ssa, options.show_brillig)?;

Ok(CompiledProgram { circuit, debug, abi })
}
6 changes: 2 additions & 4 deletions crates/noirc_evaluator/src/ssa_refactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ pub mod ssa_gen;
/// convert the final SSA into ACIR and return it.
pub(crate) fn optimize_into_acir(
program: Program,
allow_log_ops: bool,
print_ssa_passes: bool,
print_brillig_trace: bool,
) -> Result<GeneratedAcir, RuntimeError> {
Expand Down Expand Up @@ -63,7 +62,7 @@ pub(crate) fn optimize_into_acir(
.dead_instruction_elimination()
.print(print_ssa_passes, "After Dead Instruction Elimination:");
}
ssa.into_acir(brillig, abi_distinctness, allow_log_ops)
ssa.into_acir(brillig, abi_distinctness)
}

/// Compiles the Program into ACIR and applies optimizations to the arithmetic gates
Expand All @@ -74,7 +73,6 @@ pub fn create_circuit(
program: Program,
enable_ssa_logging: bool,
enable_brillig_logging: bool,
show_output: bool,
) -> Result<(Circuit, DebugInfo, Abi), RuntimeError> {
let func_sig = program.main_function_signature.clone();
let GeneratedAcir {
Expand All @@ -84,7 +82,7 @@ pub fn create_circuit(
locations,
input_witnesses,
..
} = optimize_into_acir(program, show_output, enable_ssa_logging, enable_brillig_logging)?;
} = optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?;

let abi = gen_abi(func_sig, &input_witnesses, return_witnesses.clone());
let public_abi = abi.clone().public_abi();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@
/// Returns the quotient and remainder such that lhs = rhs * quotient + remainder
/// and |remainder| < |rhs|
/// and remainder has the same sign than lhs
/// Note that this is not the euclidian division, where we have instead remainder < |rhs|

Check warning on line 502 in crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (euclidian)
///
///
///
Expand Down Expand Up @@ -608,7 +608,7 @@
}

/// Returns an `AcirVar` which will be constrained to be lhs mod 2^{rhs}
/// In order to do this, we simply perform euclidian division of lhs by 2^{rhs}

Check warning on line 611 in crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (euclidian)
/// The remainder of the division is then lhs mod 2^{rhs}
pub(crate) fn truncate_var(
&mut self,
Expand Down Expand Up @@ -827,19 +827,6 @@
self.radix_decompose(endian, input_var, two_var, limb_count_var, result_element_type)
}

/// Prints the given `AcirVar`s as witnesses.
pub(crate) fn print(&mut self, input: Vec<AcirValue>) -> Result<(), RuntimeError> {
let input = Self::flatten_values(input);

let witnesses = vecmap(input, |acir_var| {
let var_data = &self.vars[&acir_var];
let expr = var_data.to_expression();
self.acir_ir.get_or_create_witness(&expr)
});
self.acir_ir.call_print(witnesses);
Ok(())
}

/// Flatten the given Vector of AcirValues into a single vector of only variables.
/// Each AcirValue::Array in the vector is recursively flattened, so each element
/// will flattened into the resulting Vec. E.g. flatten_values([1, [2, 3]) == [1, 2, 3].
Expand Down Expand Up @@ -963,7 +950,7 @@
}

/// Recursively create acir values for returned arrays. This is necessary because a brillig returned array can have nested arrays as elements.
/// A singular array of witnesses is collected for a top level array, by deflattening the assigned witnesses at each level.

Check warning on line 953 in crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (deflattening)
fn brillig_array_output(
&mut self,
element_types: &[AcirType],
Expand Down
39 changes: 10 additions & 29 deletions crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,9 @@ impl Ssa {
self,
brillig: Brillig,
abi_distinctness: AbiDistinctness,
allow_log_ops: bool,
) -> Result<GeneratedAcir, RuntimeError> {
let context = Context::new();
let mut generated_acir = context.convert_ssa(self, brillig, allow_log_ops)?;
let mut generated_acir = context.convert_ssa(self, brillig)?;

match abi_distinctness {
AbiDistinctness::Distinct => {
Expand Down Expand Up @@ -144,15 +143,10 @@ impl Context {
}

/// Converts SSA into ACIR
fn convert_ssa(
self,
ssa: Ssa,
brillig: Brillig,
allow_log_ops: bool,
) -> Result<GeneratedAcir, RuntimeError> {
fn convert_ssa(self, ssa: Ssa, brillig: Brillig) -> Result<GeneratedAcir, RuntimeError> {
let main_func = ssa.main();
match main_func.runtime() {
RuntimeType::Acir => self.convert_acir_main(main_func, &ssa, brillig, allow_log_ops),
RuntimeType::Acir => self.convert_acir_main(main_func, &ssa, brillig),
RuntimeType::Brillig => self.convert_brillig_main(main_func, brillig),
}
}
Expand All @@ -162,14 +156,13 @@ impl Context {
main_func: &Function,
ssa: &Ssa,
brillig: Brillig,
allow_log_ops: bool,
) -> Result<GeneratedAcir, RuntimeError> {
let dfg = &main_func.dfg;
let entry_block = &dfg[main_func.entry_block()];
let input_witness = self.convert_ssa_block_params(entry_block.parameters(), dfg)?;

for instruction_id in entry_block.instructions() {
self.convert_ssa_instruction(*instruction_id, dfg, ssa, &brillig, allow_log_ops)?;
self.convert_ssa_instruction(*instruction_id, dfg, ssa, &brillig)?;
}

self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?;
Expand Down Expand Up @@ -294,7 +287,6 @@ impl Context {
dfg: &DataFlowGraph,
ssa: &Ssa,
brillig: &Brillig,
allow_log_ops: bool,
) -> Result<(), RuntimeError> {
let instruction = &dfg[instruction_id];
self.acir_context.set_location(dfg.get_location(&instruction_id));
Expand Down Expand Up @@ -339,13 +331,8 @@ impl Context {
}
}
Value::Intrinsic(intrinsic) => {
let outputs = self.convert_ssa_intrinsic_call(
*intrinsic,
arguments,
dfg,
allow_log_ops,
result_ids,
)?;
let outputs = self
.convert_ssa_intrinsic_call(*intrinsic, arguments, dfg, result_ids)?;

// Issue #1438 causes this check to fail with intrinsics that return 0
// results but the ssa form instead creates 1 unit result value.
Expand Down Expand Up @@ -936,7 +923,6 @@ impl Context {
intrinsic: Intrinsic,
arguments: &[ValueId],
dfg: &DataFlowGraph,
allow_log_ops: bool,
result_ids: &[ValueId],
) -> Result<Vec<AcirValue>, RuntimeError> {
match intrinsic {
Expand Down Expand Up @@ -966,13 +952,8 @@ impl Context {

self.acir_context.bit_decompose(endian, field, bit_size, result_type)
}
Intrinsic::Println => {
let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
if allow_log_ops {
self.acir_context.print(inputs)?;
}
Ok(Vec::new())
}
// TODO(#2115): Remove the println intrinsic as the oracle println is now used instead
Intrinsic::Println => Ok(Vec::new()),
Intrinsic::Sort => {
let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
// We flatten the inputs and retrieve the bit_size of the elements
Expand Down Expand Up @@ -1002,7 +983,7 @@ impl Context {
}
Intrinsic::ArrayLen => {
let len = match self.convert_value(arguments[0], dfg) {
AcirValue::Var(_, _) => unreachable!("Non-array passed to array.len() method"),
AcirValue::Var(_, _) => unreachable!("Non-array passed to array.len() method"),
AcirValue::Array(values) => (values.len() as u128).into(),
AcirValue::DynamicArray(array) => (array.len as u128).into(),
};
Expand Down Expand Up @@ -1140,7 +1121,7 @@ mod tests {
let ssa = builder.finish();

let context = Context::new();
let acir = context.convert_ssa(ssa, Brillig::default(), false).unwrap();
let acir = context.convert_ssa(ssa, Brillig::default()).unwrap();

let expected_opcodes =
vec![Opcode::Arithmetic(&Expression::one() - &Expression::from(Witness(1)))];
Expand Down
4 changes: 2 additions & 2 deletions crates/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ pub fn compile(args: JsValue) -> JsValue {
<JsValue as JsValueSerdeExt>::from_serde(&optimized_contracts).unwrap()
} else {
let main = context.get_main_function(&crate_id).expect("Could not find main function!");
let mut compiled_program = compile_no_check(&context, true, &options.compile_options, main)
.expect("Compilation failed");
let mut compiled_program =
compile_no_check(&context, &options.compile_options, main).expect("Compilation failed");

compiled_program.circuit = optimize_circuit(compiled_program.circuit);

Expand Down