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: Do not create new memory block when not needed #2142

Merged
merged 7 commits into from
Aug 4, 2023
Merged

chore: Do not create new memory block when not needed #2142

merged 7 commits into from
Aug 4, 2023

Conversation

guipublic
Copy link
Contributor

Description

Problem*

Array_set was always creating a new copy of an acir MemoryBlock, because it creates a new ssa array

Resolves #2014

Summary*

We implement a new ssa pass which maps ssa arrays with their last use.
In acir-gen, when we process the instruction corresponding to the last use of an array, we can avoid to create a new one and copy it, and we can directly operate on that array.
In order to do this, we take benefit from the ssa array-> acir block Id map and map the new ssa array to the block ID of the original ssa array.

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.

@jfecher jfecher added this pull request to the merge queue Aug 4, 2023
@TomAFrench TomAFrench changed the title chore: Do not create new MemoryBlock when not needed chore: Do not create new MemoryBlock when not needed Aug 4, 2023
@TomAFrench TomAFrench changed the title chore: Do not create new MemoryBlock when not needed chore: Do not create new memory block when not needed Aug 4, 2023
Merged via the queue into master with commit bbf7ae2 Aug 4, 2023
@jfecher jfecher deleted the gd/issue_2014 branch August 4, 2023 15:50
TomAFrench added a commit that referenced this pull request Aug 7, 2023
@TomAFrench
Merge branch 'master' into jb/noir-testing-workflow
1b7eca9 
* master:
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
TomAFrench added a commit that referenced this pull request Aug 7, 2023
@TomAFrench
Merge branch 'master' into assert-eq-builtin
8bd4cbd 
* master: (35 commits)
  feat: Issue warning for signed integers (#2185)
  chore: Add `noir_wasm` testing workflow (#1921)
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@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.

Make dynamic array in acir gen efficient
2 participants
@guipublic @jfecher