Skip to content

Commit

Permalink
feat: Handle ACIR calls in the debugger (#5051)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4824

With the introduction of ACIR calls to allow non-inlined ACIR functions,
the debugger did not have proper support for programs that had them,
neither when compiling in ACIR mode, nor in the default mode which
compiles the full program to Brillig.

## Summary\*

This PR depends on #4941 and is built on top of it.

The changes allow compiling a program with `#[fold]` (and other
non-inline) annotated functions in Brillig mode (the default when
debugging) by forcing the selection of the Brillig runtime when
compiling them. For inline functions, during SSA generation the runtime
is _not changed_ in order to allow some optimizations in the stdlib to
still work. For example, [some constant
assertions](https://github.com/noir-lang/noir/blob/9c6de4b25d318c6d211361dd62a112a9d2432c56/noir_stdlib/src/field.nr#L6)
don't work unless the function is inlined.

When debugging in ACIR mode (with the `--acir-mode` CLI or
`generateAcir: true` DAP options), the debugger now properly handles
programs with multiple circuits and ACIR calls by:

1. keeping a stack of ACVMs and a record of the circuit being executed
by each one, and
2. extending the debug location to also indicate in which circuit is the
current opcode being executed

## Additional Context


## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Martin Verzilli <martin.verzilli@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Ana Perez Ghiglia <aghiglia@manas.tech>
  • Loading branch information
4 people authored Jul 12, 2024
1 parent 2947aba commit 0541568
Show file tree
Hide file tree
Showing 11 changed files with 633 additions and 346 deletions.
19 changes: 13 additions & 6 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,19 @@ pub fn create_program(
let recursive = program.recursive;
let ArtifactsAndWarnings((generated_acirs, generated_brillig, error_types), ssa_level_warnings) =
optimize_into_acir(program, options)?;
assert_eq!(
generated_acirs.len(),
func_sigs.len(),
"The generated ACIRs should match the supplied function signatures"
);

if options.force_brillig_output {
assert_eq!(
generated_acirs.len(),
1,
"Only the main ACIR is expected when forcing Brillig output"
);
} else {
assert_eq!(
generated_acirs.len(),
func_sigs.len(),
"The generated ACIRs should match the supplied function signatures"
);
}
let mut program_artifact = SsaProgramArtifact::new(generated_brillig, error_types);

// Add warnings collected at the Ssa stage
Expand Down
11 changes: 8 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use acvm::{acir::AcirField, FieldElement};
use iter_extended::vecmap;
use noirc_errors::Location;
use noirc_frontend::ast::{BinaryOpKind, Signedness};
use noirc_frontend::monomorphization::ast::{self, LocalId, Parameters};
use noirc_frontend::monomorphization::ast::{self, InlineType, LocalId, Parameters};
use noirc_frontend::monomorphization::ast::{FuncId, Program};

use crate::errors::RuntimeError;
Expand Down Expand Up @@ -121,9 +121,14 @@ impl<'a> FunctionContext<'a> {
///
/// Note that the previous function cannot be resumed after calling this. Developers should
/// avoid calling new_function until the previous function is completely finished with ssa-gen.
pub(super) fn new_function(&mut self, id: IrFunctionId, func: &ast::Function) {
pub(super) fn new_function(
&mut self,
id: IrFunctionId,
func: &ast::Function,
force_brillig_runtime: bool,
) {
self.definitions.clear();
if func.unconstrained {
if func.unconstrained || (force_brillig_runtime && func.inline_type != InlineType::Inline) {
self.builder.new_brillig_function(func.name.clone(), id);
} else {
self.builder.new_function(func.name.clone(), id, func.inline_type);
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub(crate) fn generate_ssa(
// to generate SSA for each function used within the program.
while let Some((src_function_id, dest_id)) = context.pop_next_function_in_queue() {
let function = &context.program[src_function_id];
function_context.new_function(dest_id, function);
function_context.new_function(dest_id, function, force_brillig_runtime);
function_context.codegen_function_body(&function.body)?;
}

Expand Down
2 changes: 1 addition & 1 deletion tooling/debugger/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ build-data.workspace = true
acvm.workspace = true
fm.workspace = true
nargo.workspace = true
noirc_frontend.workspace = true
noirc_frontend = { workspace = true, features = ["bn254"] }
noirc_printable_type.workspace = true
noirc_errors.workspace = true
noirc_driver.workspace = true
Expand Down
8 changes: 0 additions & 8 deletions tooling/debugger/ignored-tests.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
brillig_references
debug_logs
fold_after_inlined_calls
fold_basic
fold_basic_nested_call
fold_call_witness_condition
fold_complex_outputs
fold_distinct_return
fold_fibonacci
fold_numeric_generic_poseidon
is_unconstrained
macros
references
Expand Down
Loading

0 comments on commit 0541568

Please sign in to comment.