Skip to content

chore: Perform Circuit optimization in nargo#1394

Merged
kevaundray merged 16 commits intomasterfrom
separate-acvm-compile-from-evaluator
Jul 11, 2023
Merged

chore: Perform Circuit optimization in nargo#1394
kevaundray merged 16 commits intomasterfrom
separate-acvm-compile-from-evaluator

Conversation

@TomAFrench
Copy link
Copy Markdown
Member

@TomAFrench TomAFrench commented May 24, 2023

Description

Problem*

This PR removes all knowledge of which backend is being used from the driver + evaluator. Generic ACIR is then returned and is optimized in nargo.

The changes to the backend interface have thrown up an annoyance in that we need to optimize the circuit before we preprocess the circuit rather than allowing the backend to handle this itself. The current situation downloads a CRS based on the size of the unoptimized circuit rather than the one the backend will actually be using.

Related to #1102

Summary*

This PR sets out to

Example

Before:


After:


Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench TomAFrench changed the title chore: separate circuit compilation and optimization chore: Separate circuit compilation and optimization May 24, 2023
@TomAFrench TomAFrench force-pushed the separate-acvm-compile-from-evaluator branch 3 times, most recently from 8c4d8bb to 29e8b9e Compare May 24, 2023 10:57
Base automatically changed from phated/acvm-0.12.0 to master May 24, 2023 12:10
@TomAFrench TomAFrench force-pushed the separate-acvm-compile-from-evaluator branch from 29e8b9e to e1debf7 Compare May 24, 2023 12:23
Comment thread crates/nargo_cli/src/cli/compile_cmd.rs Outdated
Copy link
Copy Markdown
Contributor

@guipublic guipublic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR breaks #1333

@TomAFrench
Copy link
Copy Markdown
Member Author

@phated Relevant to LSP conversation.

@TomAFrench
Copy link
Copy Markdown
Member Author

Merged something to fix all of the merge conflicts. I'm getting test failures which correspond to unoptimized ACIR trying to be passed to bberg somewhere which we need to address.

@TomAFrench TomAFrench changed the title chore: Separate circuit compilation and optimization chore: Perform Circuit optimization in nargo Jul 11, 2023
@TomAFrench TomAFrench requested a review from kevaundray July 11, 2023 00:22
@TomAFrench
Copy link
Copy Markdown
Member Author

TomAFrench commented Jul 11, 2023

This PR pulls the ACIR optimization up from noirc_driver to nargo_cli so that we can start thinking of ACIR optimization as a separate process to ACIR generation.

This is desirable for a number of reasons, including:

  • Generation of a backend-agnostic test suite of ACIR programs + initial witnesses which various backends can test against.
  • Simplification of the user flow with pluggable backends as optimisation can be delegated to the backend which removes a round of communication.

Comment thread crates/nargo_cli/src/cli/compile_cmd.rs
Comment thread crates/nargo_cli/src/cli/compile_cmd.rs
@kevaundray kevaundray dismissed guipublic’s stale review July 11, 2023 01:25

moving towards compilation then optimization being separate so we can eventually compile without a backend

@TomAFrench TomAFrench marked this pull request as ready for review July 11, 2023 01:58
@kevaundray kevaundray added this pull request to the merge queue Jul 11, 2023
Merged via the queue into master with commit 2e835b4 Jul 11, 2023
@kevaundray kevaundray deleted the separate-acvm-compile-from-evaluator branch July 11, 2023 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants