Address GtEq extra opcodes #1444
Labels
Comments
5 tasks
kevaundray
added a commit
that referenced
this issue
May 30, 2023
|
Related to this @jfecher noted we can switch the arguments instead of flipping the results: #1433 (comment) |
|
That comment was a mistake, see #1433 (comment), we cannot just swap the arguments, we do need a not operation. |
kevaundray
added a commit
that referenced
this issue
May 30, 2023
* 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 * Addressed in Address GtEq extra opcodes #1444
kevaundray
added a commit
that referenced
this issue
May 30, 2023
…rVar more explicit/mandatory (#1436) * 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. * Addressed in Address GtEq extra opcodes #1444
kevaundray
added a commit
that referenced
this issue
May 31, 2023
* 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 * Addressed in Address GtEq extra opcodes #1444 * chore(ssa refactor): acir-gen for ! instruction * inline function * 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): return acir var/result pair for binding --------- Co-authored-by: Kevaundray Wedderburn <kevtheappdev@gmail.com>
kevaundray
added a commit
that referenced
this issue
Jun 1, 2023
… 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>
kevaundray
added a commit
that referenced
this issue
Jun 2, 2023
* 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>
Closed
jfecher
added a commit
that referenced
this issue
Jun 2, 2023
* 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>
|
We now simplify double nots to just refer to the original value. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
New ACIR gen effectively constrains
Ltas notGtEq. However, the new SSA IR has noGtEqop, and represents it as notLt. Therefore a program containing>=effectively constrains it as not notGtEq.Happy Case
Either optimise away the redundant ACIR expressions, or avoid their creation by adding
GtEqto the SSA IR.Alternatives Considered
No response
Additional Context
No response
Would you like to submit a PR for this Issue?
No
Support Needs
No response
The text was updated successfully, but these errors were encountered: