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.

Sign up for GitHub

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

Jump to bottom

chore(ssa refactor): add range constraints for array param elements #1460

Merged
merged 14 commits into from
Jun 1, 2023
Merged

chore(ssa refactor): add range constraints for array param elements #1460

merged 14 commits into from
Jun 1, 2023

Conversation

joss-aztec
Copy link
Contributor

Description

Problem*

Resolves #1456

Summary*

This PR sets out to add range constraints for each array element that appears in the parameters to main

Example

fn main(xs : [u8; 2]) -> pub u8 {
    xs[1]
}

compiles to

current witness index : 3
public parameters indices : []
return value indices : [2]
BLACKBOX::RANGE [(_1, num_bits: 8)] [ ]
BLACKBOX::RANGE [(_2, num_bits: 8)] [ ]

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.

@joss-aztec joss-aztec mentioned this pull request May 31, 2023
Merged
5 tasks
jfecher
jfecher approved these changes May 31, 2023
View reviewed changes
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

I'd like to remove this code eventually, but I think it is fine for now until we decide how we want to restructure the Reference type in SSA to fix this and the array lengths issue

@kevaundray kevaundray mentioned this pull request May 31, 2023
Closed
@kevaundray
Copy link
Contributor

I've opened up an issue so that we come back to this #1469

Base automatically changed from joss/memory to master May 31, 2023 20:19
@kevaundray kevaundray changed the base branch from master to kw/ssa-refactor-only-tests June 1, 2023 11:05
@kevaundray kevaundray enabled auto-merge June 1, 2023 11:34
@kevaundray kevaundray added this pull request to the merge queue Jun 1, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 1, 2023
@kevaundray kevaundray added this pull request to the merge queue Jun 1, 2023
@kevaundray kevaundray removed this pull request from the merge queue due to the queue being cleared Jun 1, 2023
@kevaundray kevaundray merged commit 3c5a747 into kw/ssa-refactor-only-tests Jun 1, 2023
@kevaundray kevaundray deleted the joss/range-constrain-array-elems branch June 1, 2023 14:38
kevaundray added a commit that referenced this pull request Jun 2, 2023
@kevaundray @jfecher
@joss-aztec @TomAFrench @guipublic
chore(ssa refactor): Base branch (#1475)
d5571ad 
* 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 <jake@aztecprotocol.com>

* 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 <jake@aztecprotocol.com>
Co-authored-by: Joss <joss@aztecprotocol.com>

* 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 <kevtheappdev@gmail.com>

* 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 <kevtheappdev@gmail.com>

* 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 <kevtheappdev@gmail.com>

* 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 <joss@aztecprotocol.com>

* 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 <15848336+TomAFrench@users.noreply.github.com>

* Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Joss <joss@aztecprotocol.com>

* 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 <kevtheappdev@gmail.com>

* 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 <kevtheappdev@gmail.com>

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Joss <joss@aztecprotocol.com>
Co-authored-by: joss-aztec <94053499+joss-aztec@users.noreply.github.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: guipublic <47281315+guipublic@users.noreply.github.com>
jfecher added a commit that referenced this pull request Jun 2, 2023
@joss-aztec @kevaundray
@jfecher @TomAFrench @guipublic
chore(ssa refactor): to_radix (#1499)
961c441 
* 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 <jake@aztecprotocol.com>

* 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 <jake@aztecprotocol.com>
Co-authored-by: Joss <joss@aztecprotocol.com>

* 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 <kevtheappdev@gmail.com>

* 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 <kevtheappdev@gmail.com>

* 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 <kevtheappdev@gmail.com>

* 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 <joss@aztecprotocol.com>

* 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 <15848336+TomAFrench@users.noreply.github.com>

* Update crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Joss <joss@aztecprotocol.com>

* 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 <kevtheappdev@gmail.com>

* 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 <kevtheappdev@gmail.com>

* chore(ssa refactor): finish radix impl

* chore(ssa refactor): PR feedback

---------

Co-authored-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: guipublic <47281315+guipublic@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevaundray kevaundray kevaundray approved these changes

@jfecher jfecher jfecher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joss-aztec @kevaundray @jfecher