chore: generate brillig opcodes for an empty function #1448

guipublic · 2023-05-30T16:32:24Z

Description

Problem*

Resolves

Summary*

This PR sets out to compile noir functions to brillig and creates a brillig acir opcode for unsafe functions.
The mechanism for doing this is added to this PR but:

brillig bytecodes definition are temporarily put inside a file because they are not available yet from ACVM
brillig compilation returns a STOP opcode for all functions, it will be implemented in a later PR.
brillig acir opcode is not added and have a todo!() instead, because the opcode is not available from ACIR.
We assume no argument nor return value from the function. They will be implemented in a later PR.

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.

crates/noirc_evaluator/src/brillig/acvm_brillig.rs

kevaundray

Leaving the architectural changes to the IR to Jake. To be mergable we should document all constants, methods and structs in the brillig directory and any new functions added

jfecher

A few comments on some naming and organization choices.

One larger comment for @kevaundray that I'll point out below.

crates/noirc_evaluator/src/brillig/acvm_brillig.rs

crates/noirc_evaluator/src/brillig/artefact.rs

crates/noirc_evaluator/src/brillig/acvm_brillig.rs

crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs

crates/noirc_evaluator/src/ssa_refactor/ir/function.rs

crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs

crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs

crates/noirc_evaluator/src/brillig/mod.rs

guipublic · 2023-06-01T16:59:31Z

I merged with the 'base branch', should the PR still target master or rather the base branch?

kevaundray · 2023-06-01T18:07:04Z

I merged with the 'base branch', should the PR still target master or rather the base branch?

I have changed the branch to target the base branch

… gd/brillig_empty_e2e

Spoke internally -- changes have been addressed

kevaundray

Theres a bit of code duplication between FunctionBuilder::new and new_function_with_typebut nothing too drastic

* comment out tests for normal code path * chore(ssa refactor): Add method to handle black box functions in Acir gen (#1437) * add field mul and div * add code to process field mul and div * add assert example * add `is_equal` constraint * add `eq_var` method for AcirVar * process `Constrain` instruction and BinaryOp::Eq * add TODO for more than the maximum number of bits * add numeric_cast_var method which constrains a variable to be equal to a NumericType * implement casting for numeric types * add simple range constraint example * add constraints for `more_than_eq` * - add more_than_eq method - This method needs to know the bit size, so we cache this information whenever we do a range constraint. We should ideally also cache it for constants too since we can figure out their bit-sizes easily * add method to process less than binary operation * add example * assign result of cast operation * add `y` as an input value * return optimized circuit * refactor code so that mapping of the ValueIds to AcirVar is more explicit. This way we need to explicitly mark instructions as not returning anything. * Add function to push intrinsic into the IR * add method to call blackbox functions using a list of AcirVar * Addressed in Address GtEq extra opcodes #1444 * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs Co-authored-by: jfecher <[email protected]> * fix clippy * add code to handle calls to intrinsics * add example of blackbox function * remove doc test * address TODO by moving most of the blackbox logic into GeneratedACIR module * refactor black box function logic -- also renamed it from intrinsics to blackbox functions * remove TODO --------- Co-authored-by: jfecher <[email protected]> Co-authored-by: Joss <[email protected]> * chore(ssa refactor): add range constraints for array param elements (#1460) * chore(ssa refactor): acir-gen arrays in main * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs * chore(ssa refactor): PR suggestions * chore(ssa refactor): simplify test into something I know the input syntax for :-/ * chore(ssa refactor): rm todo (now has issue) * chore(ssa refactor): rm unused * chore(ssa refactor): abi_gen comment * chore(ssa refactor): split convert_ssa_load * chore(ssa refactor): more comments * chore(ssa refactor): more comments * chore(ssa refactor): add range constraints for array param elements * chore(ssa refactor): rename vars --------- Co-authored-by: kevaundray <[email protected]> * chore(ssa refactor): hookup alloc + store instructions (#1464) * chore(ssa refactor): acir-gen arrays in main * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs * chore(ssa refactor): PR suggestions * chore(ssa refactor): simplify test into something I know the input syntax for :-/ * chore(ssa refactor): rm todo (now has issue) * chore(ssa refactor): rm unused * chore(ssa refactor): abi_gen comment * chore(ssa refactor): split convert_ssa_load * chore(ssa refactor): more comments * chore(ssa refactor): more comments * chore(ssa refactor): hookup alloc + store instructions * chore(ssa refactor): rm assert * chore(ssa refactor): rm double insert * Revert "chore(ssa refactor): rm double insert" This reverts commit f47a75f. * chore(ssa refactor): rm double insert --------- Co-authored-by: kevaundray <[email protected]> * chore(ssa refactor): bitwise ops (rebased) (#1477) * chore(ssa refactor): bitwise ops * Update crates/nargo_cli/tests/test_data_ssa_refactor/simple_bitwise/Prover.toml * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs * chore(ssa refactor): fix OR impl + tidy up * chore(ssa refactor): remove XOR & AND 1-bit opt; Assert equal bit size * chore(ssa refactor): PR feedback --------- Co-authored-by: kevaundray <[email protected]> * chore(ssa refactor): Adds a regression test for repeated op (#1470) * add test for adding same constant * chore(ssa refactor): add_data dupe fix * chore(ssa refactor): update regression test and remove redundant dupe check * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs * make test simpler --------- Co-authored-by: Joss <[email protected]> * chore(ssa refactor): Add code to handle left and right shifts (#1471) * add shift left and shift right in AcirVar * call shl and shr in `convert_ssa_binary` * add example * remove TODO * update todos * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs Co-authored-by: Tom French <[email protected]> * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs Co-authored-by: Tom French <[email protected]> --------- Co-authored-by: Tom French <[email protected]> Co-authored-by: jfecher <[email protected]> Co-authored-by: Joss <[email protected]> * chore(ssa refactor): blackbox array arg support (#1484) * chore(ssa refactor): blackbox array arg support * chore(ssa refactor): fix post merge * chore(ssa refactor): push array flattening up * chore(ssa refactor): fix PR nits * chore(ssa refactor): ACIR-gen log directives (#1488) * chore(ssa refactor): blackbox array arg support * chore(ssa refactor): fix post merge * chore(ssa refactor): push array flattening up * chore(ssa refactor): ACIR-gen log directives * add back tests * chore: generate brillig opcodes for an empty function (#1448) * Brillig almost e2e for empty function * Code review * Donot inline brillig functions * decouple brillig from ssa * Add doc comments * convert only brillig functions into brillig * fix typo * add `new_function` and `new_brillig_function` * refactor code to match * fix clippy * call intrinsic directly via match --------- Co-authored-by: Kevaundray Wedderburn <[email protected]> * chore(ssa refactor): Remove dead blackboxes and handle pedersen domain separator (#1503) * chore(ssa refactor): Rm dead blackboxes and handle pedersen domain separator * chore(ssa refactor): variable keccak * chore(ssa refactor): switch between var/fixed keccak * use simpler case for now * remove Keccak message_size logic --------- Co-authored-by: Kevaundray Wedderburn <[email protected]> --------- Co-authored-by: jfecher <[email protected]> Co-authored-by: Joss <[email protected]> Co-authored-by: joss-aztec <[email protected]> Co-authored-by: Tom French <[email protected]> Co-authored-by: guipublic <[email protected]>

* comment out tests for normal code path * chore(ssa refactor): Add method to handle black box functions in Acir gen (#1437) * add field mul and div * add code to process field mul and div * add assert example * add `is_equal` constraint * add `eq_var` method for AcirVar * process `Constrain` instruction and BinaryOp::Eq * add TODO for more than the maximum number of bits * add numeric_cast_var method which constrains a variable to be equal to a NumericType * implement casting for numeric types * add simple range constraint example * add constraints for `more_than_eq` * - add more_than_eq method - This method needs to know the bit size, so we cache this information whenever we do a range constraint. We should ideally also cache it for constants too since we can figure out their bit-sizes easily * add method to process less than binary operation * add example * assign result of cast operation * add `y` as an input value * return optimized circuit * refactor code so that mapping of the ValueIds to AcirVar is more explicit. This way we need to explicitly mark instructions as not returning anything. * Add function to push intrinsic into the IR * add method to call blackbox functions using a list of AcirVar * Addressed in Address GtEq extra opcodes #1444 * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs Co-authored-by: jfecher <[email protected]> * fix clippy * add code to handle calls to intrinsics * add example of blackbox function * remove doc test * address TODO by moving most of the blackbox logic into GeneratedACIR module * refactor black box function logic -- also renamed it from intrinsics to blackbox functions * remove TODO --------- Co-authored-by: jfecher <[email protected]> Co-authored-by: Joss <[email protected]> * chore(ssa refactor): add range constraints for array param elements (#1460) * chore(ssa refactor): acir-gen arrays in main * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs * chore(ssa refactor): PR suggestions * chore(ssa refactor): simplify test into something I know the input syntax for :-/ * chore(ssa refactor): rm todo (now has issue) * chore(ssa refactor): rm unused * chore(ssa refactor): abi_gen comment * chore(ssa refactor): split convert_ssa_load * chore(ssa refactor): more comments * chore(ssa refactor): more comments * chore(ssa refactor): add range constraints for array param elements * chore(ssa refactor): rename vars --------- Co-authored-by: kevaundray <[email protected]> * chore(ssa refactor): hookup alloc + store instructions (#1464) * chore(ssa refactor): acir-gen arrays in main * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/memory.rs * chore(ssa refactor): PR suggestions * chore(ssa refactor): simplify test into something I know the input syntax for :-/ * chore(ssa refactor): rm todo (now has issue) * chore(ssa refactor): rm unused * chore(ssa refactor): abi_gen comment * chore(ssa refactor): split convert_ssa_load * chore(ssa refactor): more comments * chore(ssa refactor): more comments * chore(ssa refactor): hookup alloc + store instructions * chore(ssa refactor): rm assert * chore(ssa refactor): rm double insert * Revert "chore(ssa refactor): rm double insert" This reverts commit f47a75f. * chore(ssa refactor): rm double insert --------- Co-authored-by: kevaundray <[email protected]> * chore(ssa refactor): bitwise ops (rebased) (#1477) * chore(ssa refactor): bitwise ops * Update crates/nargo_cli/tests/test_data_ssa_refactor/simple_bitwise/Prover.toml * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs * chore(ssa refactor): fix OR impl + tidy up * chore(ssa refactor): remove XOR & AND 1-bit opt; Assert equal bit size * chore(ssa refactor): PR feedback --------- Co-authored-by: kevaundray <[email protected]> * chore(ssa refactor): Adds a regression test for repeated op (#1470) * add test for adding same constant * chore(ssa refactor): add_data dupe fix * chore(ssa refactor): update regression test and remove redundant dupe check * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs * make test simpler --------- Co-authored-by: Joss <[email protected]> * chore(ssa refactor): Add code to handle left and right shifts (#1471) * add shift left and shift right in AcirVar * call shl and shr in `convert_ssa_binary` * add example * remove TODO * update todos * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs Co-authored-by: Tom French <[email protected]> * Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs Co-authored-by: Tom French <[email protected]> --------- Co-authored-by: Tom French <[email protected]> Co-authored-by: jfecher <[email protected]> Co-authored-by: Joss <[email protected]> * chore(ssa refactor): blackbox array arg support (#1484) * chore(ssa refactor): blackbox array arg support * chore(ssa refactor): fix post merge * chore(ssa refactor): push array flattening up * chore(ssa refactor): fix PR nits * chore(ssa refactor): ACIR-gen log directives (#1488) * chore(ssa refactor): blackbox array arg support * chore(ssa refactor): fix post merge * chore(ssa refactor): push array flattening up * chore(ssa refactor): ACIR-gen log directives * chore(ssa refactor): to_radix * add back tests * chore: generate brillig opcodes for an empty function (#1448) * Brillig almost e2e for empty function * Code review * Donot inline brillig functions * decouple brillig from ssa * Add doc comments * convert only brillig functions into brillig * fix typo * add `new_function` and `new_brillig_function` * refactor code to match * fix clippy * call intrinsic directly via match --------- Co-authored-by: Kevaundray Wedderburn <[email protected]> * chore(ssa refactor): Remove dead blackboxes and handle pedersen domain separator (#1503) * chore(ssa refactor): Rm dead blackboxes and handle pedersen domain separator * chore(ssa refactor): variable keccak * chore(ssa refactor): switch between var/fixed keccak * use simpler case for now * remove Keccak message_size logic --------- Co-authored-by: Kevaundray Wedderburn <[email protected]> * chore(ssa refactor): finish radix impl * chore(ssa refactor): PR feedback --------- Co-authored-by: Kevaundray Wedderburn <[email protected]> Co-authored-by: jfecher <[email protected]> Co-authored-by: Tom French <[email protected]> Co-authored-by: guipublic <[email protected]>

Brillig almost e2e for empty function

47711ea

kevaundray reviewed May 30, 2023

View reviewed changes

crates/noirc_evaluator/src/brillig/acvm_brillig.rs Show resolved Hide resolved

kevaundray reviewed May 30, 2023

View reviewed changes

jfecher previously requested changes May 30, 2023

View reviewed changes

guipublic added 5 commits May 31, 2023 08:14

Code review

37bdca6

Donot inline brillig functions

1df2467

decouple brillig from ssa

a91dfdc

Add doc comments

dcd0fd6

convert only brillig functions into brillig

664d1f1

kevaundray reviewed Jun 1, 2023

View reviewed changes

crates/noirc_evaluator/src/brillig/mod.rs Outdated Show resolved Hide resolved

Merge branch 'kw/ssa-refactor-only-tests' into gd/brillig_empty_e2e

3fac3f6

fix typo

a021f9a

kevaundray changed the base branch from master to kw/ssa-refactor-only-tests June 1, 2023 18:05

kevaundray added 3 commits June 2, 2023 00:15

Merge remote-tracking branch 'origin/kw/ssa-refactor-only-tests' into…

27c2433

… gd/brillig_empty_e2e

add new_function and new_brillig_function

5a795c0

refactor code to match

2f56573

kevaundray approved these changes Jun 1, 2023

View reviewed changes

kevaundray marked this pull request as ready for review June 1, 2023 23:30

kevaundray added 3 commits June 2, 2023 00:43

fix clippy

2095c11

call intrinsic directly via match

de8355f

Merge branch 'kw/ssa-refactor-only-tests' into gd/brillig_empty_e2e

e5f60ba

joss-aztec merged commit 67f53ec into kw/ssa-refactor-only-tests Jun 2, 2023

joss-aztec deleted the gd/brillig_empty_e2e branch June 2, 2023 08:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: generate brillig opcodes for an empty function #1448

chore: generate brillig opcodes for an empty function #1448

guipublic commented May 30, 2023 •

edited

Loading

kevaundray left a comment

jfecher left a comment

guipublic commented Jun 1, 2023

kevaundray commented Jun 1, 2023

kevaundray left a comment

chore: generate brillig opcodes for an empty function #1448

chore: generate brillig opcodes for an empty function #1448

Conversation

guipublic commented May 30, 2023 • edited Loading

Description

Problem*

Summary*

Example

Documentation

Additional Context

PR Checklist*

kevaundray left a comment

Choose a reason for hiding this comment

jfecher left a comment

Choose a reason for hiding this comment

guipublic commented Jun 1, 2023

kevaundray commented Jun 1, 2023

kevaundray left a comment

Choose a reason for hiding this comment

guipublic commented May 30, 2023 •

edited

Loading