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 5 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
9 changes: 6 additions & 3 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,17 @@ fn run_test<B: Backend>(
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);
Expand Down
20 changes: 17 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,30 @@ 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 failed_constraint(hex_as_field: Field) {
// Note that when this method is called from a test method
// a `Failed constraint` compile error will be caught before this `println`
// is executed as the input will be a constant.
// In order to debug failing constraints with `println` calls `nargo execute`
// should be used instead
std::println(hex_as_field);
assert(hex_as_field != 0x41);
}

#[test]
fn test_failed_constraint() {
failed_constraint(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 @@ -192,7 +192,7 @@ pub fn compile_main(
}
};

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):");
Expand Down Expand Up @@ -259,7 +259,7 @@ fn compile_contract(
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 @@ fn compile_contract(
#[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 @@ -827,19 +827,6 @@ impl AcirContext {
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
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