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

feat(brillig): runtime memory allocation #1732

Merged
merged 6 commits into from
Jun 16, 2023
Merged

feat(brillig): runtime memory allocation #1732

merged 6 commits into from
Jun 16, 2023

Conversation

sirasistant
Copy link
Contributor

Description

Problem*

Resolves

Summary*

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.

@sirasistant sirasistant changed the base branch from master to kw/brillig-main June 16, 2023 11:50
@sirasistant sirasistant requested a review from kevaundray June 16, 2023 12:16
@sirasistant sirasistant marked this pull request as ready for review June 16, 2023 12:16
@sirasistant sirasistant requested a review from ludamad June 16, 2023 12:16
ludamad
ludamad reviewed Jun 16, 2023
View reviewed changes
crates/noirc_evaluator/src/brillig/brillig_ir.rs Outdated
self.push_opcode(BrilligOpcode::BinaryIntOp {
destination: ReservedRegisters::alloc(),
op: BinaryIntOp::Add,
bit_size: BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I rather use a standard 64 bit in case these semantics ever get optimized and I dont think we lose anything, just my 2 cents

ludamad
ludamad reviewed Jun 16, 2023
View reviewed changes
crates/noirc_evaluator/src/brillig/brillig_ir.rs Outdated
// Registers reserved in runtime for special purposes.
pub(crate) enum ReservedRegisters {
/// This register stores the free memory pointer. Allocations must be done after this pointer.
Alloc = 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I currently think of Alloc = StackPointer by analogy with https://stackoverflow.com/questions/1395591/what-is-exactly-the-base-pointer-and-stack-pointer-to-what-do-they-point

and https://man7.org/linux/man-pages/man3/alloca.3.html which uses the stack pointer

@kevaundray kevaundray merged commit 7b999e8 into kw/brillig-main Jun 16, 2023
@kevaundray kevaundray deleted the arv/runtime_alloc branch June 16, 2023 15:05
kevaundray added a commit that referenced this pull request Jun 17, 2023
@kevaundray @sirasistant
@ludamad @guipublic @jfecher @vezenovm @joss-aztec @TomAFrench @phated
chore(ssa refactor): SSA refactor main (#1628)
3467aab 
* disable mac tests

* feat: added support for modulo ops on brillig (#1621)

* feat: added support for modulo ops

* chore: use ACVM 0.14.3

* revert change as its unrelated

* small comment refactor

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* feat(brillig): foreign call/oracle compilation (#1600)

* remove mac runner

* chore: generate brillig opcode for simple identity unconstrained function (#1536)

* feat(brillig): added arithmetic operations on brillig (#1565)

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* make ranges be polymorphic integers

* chore(brillig): Clean up handling of Binary operations (#1571)

* chore(ssa refactor): Rename Brillig example (#1563)

* chore(brillig): added tests for all field binary operations (#1586)

* chore(brillig): added tests for brillig integer operations (#1590)

* feat: process blocks and jumps when compiling brillig (#1591)

* process jumps between blocks

* fix jumps

* add doc comments

* cargo fmt

* code refactor

* update code comment

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* feat: process blocks and jumps when compiling brillig (#1591)

* process jumps between blocks

* fix jumps

* add doc comments

* cargo fmt

* code refactor

* update code comment

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* feat(brillig): start of oracles/foreign calls

* fix: broken tests

* feat(brillig): parsing oracles/foreign calls (#1596)

* feat(brillig): start of oracles/foreign calls

* fix: broken tests

* Update execute.rs

* feat: more foreign call work

* self.data -> self.vars

* chore(brillig): Add handling of the not instruction (#1609)

* test: brillig oracle

* Reinstate option

* make behavior consistent

* remove closure

* change index_type

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

* feat(brillig): loops (#1610)

* make ranges be polymorphic integers

* feat: brillig loop support

* fix: fixed brillig returns and stop

* fix: do not apply constants folding to brillig fns

* chore: update acvm pointer, cleanup

* style: newline on cargo toml

* make behavior consistent

* remove closure

* change index_type

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

* better debug information for unsupported instruction

* remove edge case for optimizations

* clippy fix

* patch infinite loop

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>

* feat: Foreign calls compiling and basic print executed in nargo (#1612)

* get foreign calls compiling and basic print executed in nargo

* cargo clipy and cargo fmt

* missing

* Update crates/noirc_evaluator/src/brillig/brillig_gen.rs

* add issue num for logging

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* chore: resolve immutable array merge differences (#1617)

* chore(ssa refactor): Switch to immutable arrays (#1578)

* Represent SSA arrays with im::Vector

* Get tests passing

* Implement assign with immutable arrays

* Add constant folding pass

* Update comments

* Clippy

* Update comment

* Update type of array

* Update crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

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

* Undo formatting changes in instruction.rs

* Massive acir_gen update

* Refactor acir array operations into a shared function

* Appease clippy

* Update to_radix and to_bits in acir_gen to return arrays

* Disable assert

* Fix convert_type for arrays

* Include AcirType in AcirValue::Var variant

* Fix black box functions

* Appease clippy

* Fix simple_radix

* Add doc comments

---------

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

* feat: Make for-loop range be a polymorphic integer instead of just Field in unconstrained functions (#1583)

* make ranges be polymorphic integers

* make behavior consistent

* remove closure

* change index_type

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>

* chore(ssa refactor): fix brillig post master merge

* chore(ssa refactor): accidental merge undelete

---------

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

* chore(ssa refactor): Add more documentation for truncation  (#1607)

* add more documentation

* small change

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

* Update .github/workflows/test.yml

* Update .github/workflows/test.yml

* remove dbg

* change printer for foreign call

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

* Update crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

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

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: guipublic <47281315+guipublic@users.noreply.github.com>
Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: joss-aztec <94053499+joss-aztec@users.noreply.github.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

* feat: add support for assert in brillig (#1603)

* remove mac runner

* chore: generate brillig opcode for simple identity unconstrained function (#1536)

* feat(brillig): added arithmetic operations on brillig (#1565)

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* make ranges be polymorphic integers

* chore(brillig): Clean up handling of Binary operations (#1571)

* chore(ssa refactor): Rename Brillig example (#1563)

* chore(brillig): added tests for all field binary operations (#1586)

* chore(brillig): added tests for brillig integer operations (#1590)

* feat: process blocks and jumps when compiling brillig (#1591)

* process jumps between blocks

* fix jumps

* add doc comments

* cargo fmt

* code refactor

* update code comment

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* feat: process blocks and jumps when compiling brillig (#1591)

* process jumps between blocks

* fix jumps

* add doc comments

* cargo fmt

* code refactor

* update code comment

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* feat(brillig): parsing oracles/foreign calls (#1596)

* feat(brillig): start of oracles/foreign calls

* fix: broken tests

* Update execute.rs

* support assert in brillig

* self.data -> self.vars

* Avoid not in the test

* chore(brillig): Add handling of the not instruction (#1609)

* make behavior consistent

* remove closure

* change index_type

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

* feat(brillig): loops (#1610)

* make ranges be polymorphic integers

* feat: brillig loop support

* fix: fixed brillig returns and stop

* fix: do not apply constants folding to brillig fns

* chore: update acvm pointer, cleanup

* style: newline on cargo toml

* make behavior consistent

* remove closure

* change index_type

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

* better debug information for unsupported instruction

* remove edge case for optimizations

* clippy fix

* patch infinite loop

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>

* chore: resolve immutable array merge differences (#1617)

* chore(ssa refactor): Switch to immutable arrays (#1578)

* Represent SSA arrays with im::Vector

* Get tests passing

* Implement assign with immutable arrays

* Add constant folding pass

* Update comments

* Clippy

* Update comment

* Update type of array

* Update crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

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

* Undo formatting changes in instruction.rs

* Massive acir_gen update

* Refactor acir array operations into a shared function

* Appease clippy

* Update to_radix and to_bits in acir_gen to return arrays

* Disable assert

* Fix convert_type for arrays

* Include AcirType in AcirValue::Var variant

* Fix black box functions

* Appease clippy

* Fix simple_radix

* Add doc comments

---------

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

* feat: Make for-loop range be a polymorphic integer instead of just Field in unconstrained functions (#1583)

* make ranges be polymorphic integers

* make behavior consistent

* remove closure

* change index_type

* Update crates/noirc_frontend/src/hir/type_check/expr.rs

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>

* chore(ssa refactor): fix brillig post master merge

* chore(ssa refactor): accidental merge undelete

---------

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

* chore(ssa refactor): Add more documentation for truncation  (#1607)

* add more documentation

* small change

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

* Update .github/workflows/test.yml

* Update .github/workflows/test.yml

* Remove optimisation for handling assert

* add the assert jump to the list of jumps to fix

* add doc comment

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: jfecher <jake@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>

* chore(ssa refactor): Separate Brillig only logic from Brillig-SSA generation logic (#1631)

* separate brillig only logic from logic that combines both brillig and ssa

* move operations related to brillig-ssa conversion to brillig_gen

* intermediate step -- make build work by leaking the abstraction

* refactor return instruction -- all of the implementation details about brillig are no longer in brillig_gen

* fix clippy

* add TODO

* move `convert_integer_mod` into BrilligIr

* encapsulate modulo operation in brillig_gen

* process binary instruction

* refactor const instruction

* make code surrounding jumps satisfy the abstraction

* move not instruction

* add foreign call instruction

* move load and store instructions to brillig_ir

* move truncate instruction

* move `mov`, `stop` and `allocate_array` methods

* fix clippy

* remove push_code method in brillig_gen

* move jump and jmpif instructions to brillig_ir

* clean up terminator instruction

* remove allow(deprecated) lint

* refactor document for link and link_with

* use .into so we can get rid of Value import

* add docs

* Update crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs

Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>

* Update crates/noirc_evaluator/src/brillig/brillig_gen.rs

* make code a bit clearer

* fix clippy

---------

Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>

* chore(ssa_refactor): Improve foreign call compilation (#1644)

* remove ForeignCall instruction from SSA

* remove unnecessary Oracle RuntimeType

* remove old comment

* feat: Brillig array inputs and outputs (#1630)

* first attempt at brillig multiple array inptus

* working array identity function for brillig

* cleanup dbgs

* remove unused imports

* remove dbg

* a little cleanup

* fix up foreign calls for array inputs and outputs

* fix outputs clippy err

* move conversion to RegisterValueOrArray to its own method

* missing &mut and TODO link

* PR comment for brillig output array types

* cleanup comment

* enable struct inputs/outputs

* cargo clippy

* chore(brillig): master into brillig main (#1663)

* fix(ssa refactor): resolve replaced value ids for printing (#1535)

* fix(ssa refactor): resolve replaced value ids for printing

* fix(ssa refactor): Expand PR #1535 to resolve ValueIds in all SSA passes (#1642)

* Expand PR

* chore(ssa refactor): more value id resolving

* chore(ssa refactor): another value id resolve

---------

Co-authored-by: Joss <joss@aztecprotocol.com>

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>

* chore(ssa refactor): enable_side_effects instruction (#1547)

* chore(ssa refactor): enable_side_effects instruction

* chore(ssa refactor): fix and document enable_side_effects insertions

* chore(ssa refactor): rm comments

* fix(ssa refactor): redundant EnableSideEffects

* chore(ssa refactor): cp working tests (#1619)

* chore(ssa gen): ssa gen truncate instruction

* chore(ssa refactor): max bit size for subtract

* Update crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* chore(ssa refactor): truncate shift left

* chore(ssa refactor): Add integer modulus when truncating subtraction

* chore(ssa refactor): clippy

* chore(ssa refactor): fix left shift max bit size

* chore(ssa refactor): cp xor test

* chore(ssa refactor): cp working tests

* chore(ssa refactor): more working tests

* chore(ssa refactor): cp working test

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>

* chore(ssa refactor): Remove unit values from SSA IR (#1646)

* Remove unit values

* Fix test

* Fix comment

* chore: Upgrade codespan dependencies (#1647)

* chore(ssa refactor): Implement dead instruction elimination pass (#1595)

* Add dead instruction elimination pass

* Enable the pass

* chore(ssa refactor): simple mut test

* chore(ssa refactor): fixup and add doc comments

* chore(ssa refactor): post merge fix

---------

Co-authored-by: Joss <joss@aztecprotocol.com>

* chore(brillig): Update acvm dependency (#1653)

chore: update acvm dep

* refactor: remove unused assign

---------

Co-authored-by: joss-aztec <94053499+joss-aztec@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: Joss <joss@aztecprotocol.com>

* feat(brillig): Added cast instruction (#1649)

* feat: added cast instruction

* docs: add comment on cast

* docs: added comment on cast instruction

* simplify convert_cast

* Alvaro to review

* feat: do casts as no ops instead

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* feat(brillig): Add unique labeling and remove relative jumps (#1652)

* feat: added unique labeling

* refactor: remove relative jumps

* feat: add section/global labeling

* pr review

* Update crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

* style: clippy

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

* fix `is_signed` method

* feat(brillig): added basic array operations (#1716)

* feat: added array operations

* docs: commented functions a bit

* Update crates/noirc_evaluator/src/brillig/brillig_ir.rs

* refactor: rename method

* refactor: simplify size calculations

* style: fmt

* feat(brillig): runtime memory allocation (#1732)

* feat: use dynamic memory allocation

* docs: update comments

* refactor: rename allocation methods

* docs: updated comment

* refactor: changes from peer review

* chore(brillig): Add a register abstraction/optimization (#1737)

* chore: brillig register abstraction

* chore: small refactor

* fix: latest_register starting value

* fix: comments

* chore: remove unused method

* fix: reintroduce a safety register alloc check

* merge fix: AcirType is now an enum

* merge fix: foreign calls

* fix: Oracle output resolution (#1740)

silly fix until we use enums for foreign call results

---------

Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>
Co-authored-by: ludamad <adam.domurad@gmail.com>
Co-authored-by: guipublic <47281315+guipublic@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.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: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: Joss <joss@aztecprotocol.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ludamad ludamad ludamad approved these changes

@kevaundray kevaundray Awaiting requested review from kevaundray

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
@sirasistant @ludamad @kevaundray