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.

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

chore(ssa): Remove mem2reg run before flattening #4415

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

vezenovm
Copy link
Contributor

Description

Problem*

Before #4240 we needed mem2reg to be run as to not panic when fetching slice lengths.

Summary*

After the linked PR we have an improved startegy for tracking slice capacities by generating a slice capacities map before merging of values. This should enable us to remove a mem2reg pass that is run before flattening.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Contributor

Changes to circuit sizes

Generated at commit: bfc5221c9a57a3ffe83062b802f3bef61b0d3b85, compared to commit: 601fd9afc502236af1db0c4492698ba2298c7501

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
nested_array_in_slice +46 ❌ +4.34% +55 ❌ +0.77%
sha2_byte -1,518 ✅ -7.76% -3,442 ✅ -3.74%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
nested_array_in_slice 1,107 (+46) +4.34% 7,230 (+55) +0.77%
sha2_byte 18,033 (-1,518) -7.76% 88,680 (-3,442) -3.74%

@vezenovm
Copy link
Contributor Author

sha2_byte -1,518 ✅ -7.76% -3,442 ✅ -3.74%

Kind of surprised by this but I'll take it

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.

Don't question it 👍

@jfecher jfecher added this pull request to the merge queue Feb 23, 2024
Merged via the queue into master with commit 16d5f18 Feb 23, 2024
47 checks passed
@jfecher jfecher deleted the mv/remove-mem2reg-run branch February 23, 2024 15:16
TomAFrench added a commit that referenced this pull request Feb 26, 2024
* master: (39 commits)
  chore: remove unwanted prints (#4419)
  fix: remove print from monomorphization pass (#4417)
  chore(ssa): Remove mem2reg run before flattening (#4415)
  feat: Add HashMap to the stdlib (#4242)
  fix!: Ban Fields in for loop indices and bitwise ops (#4376)
  chore: Add #[recursive] Explainer to Documentation (#4399)
  feat(ci): Use wasm-opt when compiling wasm packages (#4334)
  fix: add handling to `noir_wasm` for projects without dependencies (#4344)
  chore: rename parameter 'filter' to 'level' in 'init_log_level' (#4403)
  chore!: bump msrv to 1.73.0 (#4406)
  chore: fix docker test workflows (#4308)
  chore: Update Vec docs (#4400)
  feat: update error message when trying to load workspace as dependency (#4393)
  chore: bump webpack dependencies (#4346)
  fix: correct invalid brillig codegen for `EmbeddedCurvePoint.add` (#4382)
  chore: remove dependency on generational-arena (#4207)
  fix(docs): update install versions (#4396)
  fix: Enforce matching types of binary ops in SSA (#4391)
  fix(docs): Update noirjs_app for 0.23 (#4378)
  chore(ci): add alerts for failed publishes (#4388)
  ...
TomAFrench added a commit that referenced this pull request Feb 26, 2024
* master:
  chore(ci): prevent msrv checks from blocking PRs (#4414)
  feat: expose separate functions to compile programs vs contracts in `noir_wasm` (#4413)
  chore: do not panic when dividing by zero (#4424)
  chore: remove unwanted prints (#4419)
  fix: remove print from monomorphization pass (#4417)
  chore(ssa): Remove mem2reg run before flattening (#4415)
  feat: Add HashMap to the stdlib (#4242)
TomAFrench added a commit that referenced this pull request Feb 26, 2024
* master: (440 commits)
  chore: remove duplicate `parse_all` function in wasm compiler (#4411)
  chore(ci): prevent msrv checks from blocking PRs (#4414)
  feat: expose separate functions to compile programs vs contracts in `noir_wasm` (#4413)
  chore: do not panic when dividing by zero (#4424)
  chore: remove unwanted prints (#4419)
  fix: remove print from monomorphization pass (#4417)
  chore(ssa): Remove mem2reg run before flattening (#4415)
  feat: Add HashMap to the stdlib (#4242)
  fix!: Ban Fields in for loop indices and bitwise ops (#4376)
  chore: Add #[recursive] Explainer to Documentation (#4399)
  feat(ci): Use wasm-opt when compiling wasm packages (#4334)
  fix: add handling to `noir_wasm` for projects without dependencies (#4344)
  chore: rename parameter 'filter' to 'level' in 'init_log_level' (#4403)
  chore!: bump msrv to 1.73.0 (#4406)
  chore: fix docker test workflows (#4308)
  chore: Update Vec docs (#4400)
  feat: update error message when trying to load workspace as dependency (#4393)
  chore: bump webpack dependencies (#4346)
  fix: correct invalid brillig codegen for `EmbeddedCurvePoint.add` (#4382)
  chore: remove dependency on generational-arena (#4207)
  ...
TomAFrench added a commit that referenced this pull request Feb 27, 2024
* master: (45 commits)
  chore(docs): correct 'Edit this page' URL for dev docs (#4433)
  feat: Sync from aztec-packages (#4390)
  chore(docs): fix external contributor force push workflow (#4437)
  chore!: Remove empty value from bounded vec (#4431)
  chore: nargo fmt (#4434)
  feat: add poseidon2 opcode implementation for acvm/brillig, and Noir (#4398)
  fix: remove panic when generic array length is not resolvable (#4408)
  chore(ci): enforce formatting of noir code in CI (#4422)
  fix: correct formatting for databus visibility types (#4423)
  chore: remove duplicate `parse_all` function in wasm compiler (#4411)
  chore(ci): prevent msrv checks from blocking PRs (#4414)
  feat: expose separate functions to compile programs vs contracts in `noir_wasm` (#4413)
  chore: do not panic when dividing by zero (#4424)
  chore: remove unwanted prints (#4419)
  fix: remove print from monomorphization pass (#4417)
  chore(ssa): Remove mem2reg run before flattening (#4415)
  feat: Add HashMap to the stdlib (#4242)
  fix!: Ban Fields in for loop indices and bitwise ops (#4376)
  chore: Add #[recursive] Explainer to Documentation (#4399)
  feat(ci): Use wasm-opt when compiling wasm packages (#4334)
  ...
TomAFrench added a commit that referenced this pull request Feb 27, 2024
* master: (46 commits)
  feat: Sync from aztec-packages (#4438)
  chore(docs): correct 'Edit this page' URL for dev docs (#4433)
  feat: Sync from aztec-packages (#4390)
  chore(docs): fix external contributor force push workflow (#4437)
  chore!: Remove empty value from bounded vec (#4431)
  chore: nargo fmt (#4434)
  feat: add poseidon2 opcode implementation for acvm/brillig, and Noir (#4398)
  fix: remove panic when generic array length is not resolvable (#4408)
  chore(ci): enforce formatting of noir code in CI (#4422)
  fix: correct formatting for databus visibility types (#4423)
  chore: remove duplicate `parse_all` function in wasm compiler (#4411)
  chore(ci): prevent msrv checks from blocking PRs (#4414)
  feat: expose separate functions to compile programs vs contracts in `noir_wasm` (#4413)
  chore: do not panic when dividing by zero (#4424)
  chore: remove unwanted prints (#4419)
  fix: remove print from monomorphization pass (#4417)
  chore(ssa): Remove mem2reg run before flattening (#4415)
  feat: Add HashMap to the stdlib (#4242)
  fix!: Ban Fields in for loop indices and bitwise ops (#4376)
  chore: Add #[recursive] Explainer to Documentation (#4399)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants