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

Debugger: Handle multiple ACIR calls #4824

Closed
vezenovm opened this issue Apr 16, 2024 · 1 comment · Fixed by #5051
Closed

Debugger: Handle multiple ACIR calls #4824

vezenovm opened this issue Apr 16, 2024 · 1 comment · Fixed by #5051
Labels
debugger enhancement New feature or request

Comments

@vezenovm
Copy link
Contributor

vezenovm commented Apr 16, 2024

Problem

With the introduction of #4428 we now support multiple ACIR function calls. Inlining remains the default, however, functions marked with #[fold] (or potentially other inline markers in the future) will be treated as separate ACIR functions. Execution has been updated in the ACVM and nargo to handle these new non-inlined methods but not in the debugger.

The relevant work that updated the codegen and execution for non-inlined ACIR can be found linked in the main issue linked above.

cc @ggiraldez @mverzilli

Happy Case

It would be nice to be able to use the debugger even when executing non-inlined ACIR. This will be especially useful as we potentially add more inline markers such as #[inline(never)].

Project Impact

Nice-to-have

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@vezenovm vezenovm added enhancement New feature or request debugger labels Apr 16, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Apr 16, 2024
@mverzilli
Copy link
Contributor

Hi @vezenovm! We're looking into this

github-merge-queue bot pushed a commit that referenced this issue Jul 12, 2024
# 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>
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugger enhancement New feature or request
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants