-
Notifications
You must be signed in to change notification settings - Fork 189
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
feat(perf): Optimize array set from get #6207
Merged
Merged
+27
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes to Brillig bytecode sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
Changes to circuit sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
asterite
approved these changes
Oct 2, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
5 tasks
TomAFrench
added a commit
that referenced
this pull request
Oct 2, 2024
* master: (36 commits) fix: ignore compression of blocks after msg.len in sha256_var (#6206) feat(perf): Optimize array set from get (#6207) chore(refactor): Array set optimization context struct for analysis (#6204) fix: type variables by default should have Any kind (#6203) feat: remove orphaned blocks from cfg to improve `simplify_cfg` pass. (#6198) fix(ssa): Check if result of array set is used in value of another array set (#6197) fix(docs): Rename recursion.md to recursion.mdx (#6195) feat: skip `remove_enable_side_effects` pass on brillig functions (#6199) feat!: Syncing TypeVariableKind with Kind (#6094) feat(perf): Simplify the cfg after DIE (#6184) feat: refactor SSA passes to run on individual functions (#6072) chore: Remove macros_api module (#6190) fix: ensure to_bytes returns the canonical decomposition (#6084) chore: rename `DefinitionKind::GenericType` (#6182) feat(perf): Remove redundant inc rc without instructions between (#6183) chore: reexport `CrateName` through `nargo` (#6177) feat(perf): Remove inc_rc instructions for arrays which are never mutably borrowed (#6168) chore(docs): Add link to more info about proving backend to Solidity verifier page (#5754) feat: let `Module::functions` and `Module::structs` return them in definition order (#6178) chore: split `test_program`s into modules (#6101) ...
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 3, 2024
🤖 I have created a release *beep* *boop* --- <details><summary>0.35.0</summary> ## [0.35.0](v0.34.0...v0.35.0) (2024-10-03) ### ⚠ BREAKING CHANGES * Syncing TypeVariableKind with Kind ([#6094](#6094)) * remove sha256 opcode (AztecProtocol/aztec-packages#4571) * add support for u1 in the avm, ToRadix's radix arg is a memory addr (AztecProtocol/aztec-packages#8570) * Infer globals to be u32 when used in a type ([#6083](#6083)) * removing implicit numeric generics ([#5837](#5837)) ### Features * (LSP) if in runtime code, always suggest functions that return Quoted as macro calls ([#6098](#6098)) ([4a160cb](4a160cb)) * (LSP) remove unused imports ([#6129](#6129)) ([98bc460](98bc460)) * (LSP) show global value on hover ([#6097](#6097)) ([3d9d072](3d9d072)) * (LSP) suggest $vars inside `quote { ... }` ([#6114](#6114)) ([73245b3](73245b3)) * Add `Expr::as_constructor` ([#5980](#5980)) ([76dea7b](76dea7b)) * Add `Expr::as_for` and `Expr::as_for_range` ([#6039](#6039)) ([abcae75](abcae75)) * Add `Expr::as_lambda` ([#6048](#6048)) ([31130dc](31130dc)) * Add a `comptime` string type for string handling at compile-time ([#6026](#6026)) ([5d2984f](5d2984f)) * Add support for u1 in the avm, ToRadix's radix arg is a memory addr (AztecProtocol/aztec-packages#8570) ([e8bbce7](e8bbce7)) * Allow silencing an unused variable defined via `let` ([#6149](#6149)) ([a2bc059](a2bc059)) * Allow visibility modifiers in struct definitions ([#6054](#6054)) ([199be58](199be58)) * Check unconstrained trait impl method matches ([#6057](#6057)) ([aedc983](aedc983)) * Default to outputting witness with file named after package ([#6031](#6031)) ([e74b4ae](e74b4ae)) * Detect unconstructed structs ([#6061](#6061)) ([bcb438b](bcb438b)) * Do not double error on import with error ([#6131](#6131)) ([9b26650](9b26650)) * Expose `derived_generators` and `pedersen_commitment_with_separator` from the stdlib ([#6154](#6154)) ([877b806](877b806)) * Faster LSP by caching file managers ([#6047](#6047)) ([c48a4f8](c48a4f8)) * Hoist constant allocation outside of loops ([#6158](#6158)) ([180bfc9](180bfc9)) * Implement `to_be_radix` in the comptime interpreter ([#6043](#6043)) ([1550278](1550278)) * Implement solver for mov_registers_to_registers ([#6089](#6089)) ([4170c55](4170c55)) * Implement type paths ([#6093](#6093)) ([2174ffb](2174ffb)) * Let `Module::functions` and `Module::structs` return them in definition order ([#6178](#6178)) ([dec9874](dec9874)) * Let LSP suggest macro calls too ([#6090](#6090)) ([26d275b](26d275b)) * Let LSP suggest trait impl methods as you are typing them ([#6029](#6029)) ([dfed81b](dfed81b)) * LSP autocompletion for `TypePath` ([#6117](#6117)) ([3f79d8f](3f79d8f)) * **metaprogramming:** Add `#[use_callers_scope]` ([#6050](#6050)) ([8c34046](8c34046)) * Optimize allocating immediate amounts of memory (AztecProtocol/aztec-packages#8579) ([e8bbce7](e8bbce7)) * Optimize constraints in sha256 ([#6145](#6145)) ([164d29e](164d29e)) * **perf:** Allow array set last uses optimization in return block of Brillig functions ([#6119](#6119)) ([5598059](5598059)) * **perf:** Handle array set optimization across blocks for Brillig functions ([#6153](#6153)) ([12cb80a](12cb80a)) * **perf:** Optimize array set from get ([#6207](#6207)) ([dfeb1c5](dfeb1c5)) * **perf:** Remove inc_rc instructions for arrays which are never mutably borrowed ([#6168](#6168)) ([a195442](a195442)) * **perf:** Remove redundant inc rc without instructions between ([#6183](#6183)) ([be9dcfe](be9dcfe)) * **perf:** Remove unused loads in mem2reg and last stores per function ([#5925](#5925)) ([19eef30](19eef30)) * **perf:** Remove useless paired RC instructions within a block during DIE ([#6160](#6160)) ([59c4118](59c4118)) * **perf:** Simplify the cfg after DIE ([#6184](#6184)) ([a1b5046](a1b5046)) * Pretty print Quoted token stream ([#6111](#6111)) ([cd81f85](cd81f85)) * Refactor SSA passes to run on individual functions ([#6072](#6072)) ([85c502c](85c502c)) * Remove aztec macros ([#6087](#6087)) ([9d96207](9d96207)) * Remove orphaned blocks from cfg to improve `simplify_cfg` pass. ([#6198](#6198)) ([b4712c5](b4712c5)) * Remove sha256 opcode (AztecProtocol/aztec-packages#4571) ([e8bbce7](e8bbce7)) * Remove unnecessary branching in keccak impl ([#6133](#6133)) ([9c69dce](9c69dce)) * Represent assertions more similarly to function calls ([#6103](#6103)) ([3ecd0e2](3ecd0e2)) * Show test output when running via LSP ([#6049](#6049)) ([9fb010e](9fb010e)) * Simplify sha256 implementation ([#6142](#6142)) ([acdfbbc](acdfbbc)) * Skip `remove_enable_side_effects` pass on brillig functions ([#6199](#6199)) ([2303615](2303615)) * **ssa:** Simplify signed casts ([#6166](#6166)) ([eec3a61](eec3a61)) * Swap endianness in-place in keccak implementation ([#6128](#6128)) ([e3cdebe](e3cdebe)) * Syncing TypeVariableKind with Kind ([#6094](#6094)) ([6440e18](6440e18)) * Visibility for globals ([#6161](#6161)) ([103b54d](103b54d)) * Visibility for modules ([#6165](#6165)) ([fcdbcb9](fcdbcb9)) * Visibility for traits ([#6056](#6056)) ([5bbd9ba](5bbd9ba)) * Visibility for type aliases ([#6058](#6058)) ([66d2a07](66d2a07)) ### Bug Fixes * (LSP) make goto and hover work well for attributes ([#6152](#6152)) ([c679bc6](c679bc6)) * Allow macros to change types on each iteration of a comptime loop ([#6105](#6105)) ([0864e7c](0864e7c)) * Allow providing default implementations of unconstrained trait methods ([#6138](#6138)) ([7679bbc](7679bbc)) * Always parse all tokens from quoted token streams ([#6064](#6064)) ([23ed74b](23ed74b)) * Be more lenient with semicolons on interned expressions ([#6062](#6062)) ([052c4fe](052c4fe)) * Consider constants as used values to keep their rc ops ([#6122](#6122)) ([1217005](1217005)) * Correct stack trace order in comptime assertion failures ([#6066](#6066)) ([04f1636](04f1636)) * Databus panic for fns with empty params (AztecProtocol/aztec-packages#8847) ([d252748](d252748)) * Decode databus return values ([#6095](#6095)) ([c40eb1f](c40eb1f)) * Disable side-effects for no_predicates functions ([#6027](#6027)) ([fc74c55](fc74c55)) * Disambiguate field or int static trait method call ([#6112](#6112)) ([5b27ea4](5b27ea4)) * Do not duplicate constant arrays in brillig ([#6155](#6155)) ([68f3022](68f3022)) * **docs:** Rename recursion.md to recursion.mdx ([#6195](#6195)) ([054e48b](054e48b)) * Don't crash on untyped global used as array length ([#6076](#6076)) ([426f295](426f295)) * Ensure to_bytes returns the canonical decomposition ([#6084](#6084)) ([b280a79](b280a79)) * Error on `&mut x` when `x` is not mutable ([#6037](#6037)) ([57afc7d](57afc7d)) * Fix canonicalization bug ([#6033](#6033)) ([7397772](7397772)) * Fix comptime type formatting ([#6079](#6079)) ([e678091](e678091)) * Handle multi-byte utf8 characters in formatter ([#6118](#6118)) ([b1d0619](b1d0619)) * Handle parenthesized expressions in array length ([#6132](#6132)) ([9f0b397](9f0b397)) * Ignore compression of blocks after msg.len in sha256_var ([#6206](#6206)) ([76eec71](76eec71)) * Infer globals to be u32 when used in a type ([#6083](#6083)) ([78262c9](78262c9)) * Initialise databus using return values ([#6074](#6074)) ([e17dfa5](e17dfa5)) * Let LSP suggest fields and methods in LValue chains ([#6051](#6051)) ([5bf6567](5bf6567)) * Let token pretty printer handle `+=` and similar token sequences ([#6135](#6135)) ([684b6cc](684b6cc)) * **mem2reg:** Remove possibility of underflow ([#6107](#6107)) ([aea5cc7](aea5cc7)) * Parse a statement as an expression ([#6040](#6040)) ([ab203e4](ab203e4)) * Pass radix directly to the blackbox ([#6164](#6164)) ([82b89c4](82b89c4)) * Preserve generic kind on trait methods ([#6099](#6099)) ([1df102a](1df102a)) * Prevent check_can_mutate crashing on undefined variable ([#6044](#6044)) ([b3accfc](b3accfc)) * Revert mistaken stack size change ([#6212](#6212)) ([a37117a](a37117a)) * **ssa:** Check if result of array set is used in value of another array set ([#6197](#6197)) ([594ec91](594ec91)) * **ssa:** RC correctness issue ([#6134](#6134)) ([5b1c896](5b1c896)) * Type variables by default should have Any kind ([#6203](#6203)) ([268f2a0](268f2a0)) * Unify macro result type with actual type ([#6086](#6086)) ([af52873](af52873)) * Update databus in flattening ([#6063](#6063)) ([e993da1](e993da1)) ### Miscellaneous Chores * Removing implicit numeric generics ([#5837](#5837)) ([eda9043](eda9043)) </details> <details><summary>0.51.0</summary> ## [0.51.0](v0.50.0...v0.51.0) (2024-10-03) ### ⚠ BREAKING CHANGES * remove sha256 opcode (AztecProtocol/aztec-packages#4571) * add support for u1 in the avm, ToRadix's radix arg is a memory addr (AztecProtocol/aztec-packages#8570) * Add Not instruction in brillig (AztecProtocol/aztec-packages#8488) * **avm:** variants for SET opcode (AztecProtocol/aztec-packages#8441) * **avm/brillig:** take addresses in calldatacopy (AztecProtocol/aztec-packages#8388) * constant inputs for blackbox (AztecProtocol/aztec-packages#7222) ### Features * (bb) 128-bit challenges (AztecProtocol/aztec-packages#8406) ([3c3ed1e](3c3ed1e)) * **acir_gen:** Width aware ACIR gen addition ([#5493](#5493)) ([85fa592](85fa592)) * Add assertions for ACVM `FunctionInput` `bit_size` ([#5864](#5864)) ([8712f4c](8712f4c)) * Add Not instruction in brillig (AztecProtocol/aztec-packages#8488) ([95e19ab](95e19ab)) * Add recursive aggregation object to proving/verification keys (AztecProtocol/aztec-packages#6770) ([4ea25db](4ea25db)) * Add reusable procedures to brillig generation (AztecProtocol/aztec-packages#7981) ([5c4f19f](5c4f19f)) * Add support for u1 in the avm, ToRadix's radix arg is a memory addr (AztecProtocol/aztec-packages#8570) ([e8bbce7](e8bbce7)) * Added indirect const instruction (AztecProtocol/aztec-packages#8065) ([5c4f19f](5c4f19f)) * Adding aggregation to honk and rollup (AztecProtocol/aztec-packages#7466) ([4ea25db](4ea25db)) * Automate verify_honk_proof input generation (AztecProtocol/aztec-packages#8092) ([5c4f19f](5c4f19f)) * **avm/brillig:** Take addresses in calldatacopy (AztecProtocol/aztec-packages#8388) ([3c3ed1e](3c3ed1e)) * **avm:** Variants for SET opcode (AztecProtocol/aztec-packages#8441) ([3c3ed1e](3c3ed1e)) * Avoid heap allocs when going to/from field (AztecProtocol/aztec-packages#7547) ([daad75c](daad75c)) * Change the layout of arrays and vectors to be a single pointer (AztecProtocol/aztec-packages#8448) ([d4832ec](d4832ec)) * Constant inputs for blackbox (AztecProtocol/aztec-packages#7222) ([fb97bb9](fb97bb9)) * Hook up secondary calldata column in dsl (AztecProtocol/aztec-packages#7759) ([4ea25db](4ea25db)) * Integrate new proving systems in e2e (AztecProtocol/aztec-packages#6971) ([daad75c](daad75c)) * Make Brillig do integer arithmetic operations using u128 instead of Bigint (AztecProtocol/aztec-packages#7518) ([daad75c](daad75c)) * Make token transfer be recursive (AztecProtocol/aztec-packages#7730) ([4ea25db](4ea25db)) * New test programs for wasm benchmarking (AztecProtocol/aztec-packages#8389) ([95e19ab](95e19ab)) * Note hashes as points (AztecProtocol/aztec-packages#7618) ([4ea25db](4ea25db)) * Optimize allocating immediate amounts of memory (AztecProtocol/aztec-packages#8579) ([e8bbce7](e8bbce7)) * Optimize constant array handling in brillig_gen (AztecProtocol/aztec-packages#7661) ([4ea25db](4ea25db)) * Optimize to_radix (AztecProtocol/aztec-packages#8073) ([5c4f19f](5c4f19f)) * Pass calldata ids to the backend (AztecProtocol/aztec-packages#7875) ([4ea25db](4ea25db)) * Poseidon2 gates for Ultra arithmetisation (AztecProtocol/aztec-packages#7494) ([5c4f19f](5c4f19f)) * **profiler:** Add support for brillig functions in opcodes-flamegraph (AztecProtocol/aztec-packages#7698) ([4ea25db](4ea25db)) * Remove sha256 opcode (AztecProtocol/aztec-packages#4571) ([e8bbce7](e8bbce7)) * Removing superfluous call to MSM (AztecProtocol/aztec-packages#7708) ([4ea25db](4ea25db)) * Report gates and VKs of private protocol circuits with megahonk (AztecProtocol/aztec-packages#7722) ([4ea25db](4ea25db)) * Simplify constant calls to `poseidon2_permutation`, `schnorr_verify` and `embedded_curve_add` ([#5140](#5140)) ([2823ba7](2823ba7)) * Small optimization in toradix (AztecProtocol/aztec-packages#8040) ([5c4f19f](5c4f19f)) * Sync from noir (AztecProtocol/aztec-packages#7392) ([fb97bb9](fb97bb9)) * Sync from noir (AztecProtocol/aztec-packages#7400) ([fb97bb9](fb97bb9)) * Sync from noir (AztecProtocol/aztec-packages#7432) ([daad75c](daad75c)) * Sync from noir (AztecProtocol/aztec-packages#7444) ([daad75c](daad75c)) * Sync from noir (AztecProtocol/aztec-packages#7454) ([daad75c](daad75c)) * Sync from noir (AztecProtocol/aztec-packages#7512) ([daad75c](daad75c)) * Sync from noir (AztecProtocol/aztec-packages#7577) ([daad75c](daad75c)) * Sync from noir (AztecProtocol/aztec-packages#7583) ([daad75c](daad75c)) * Sync from noir (AztecProtocol/aztec-packages#7743) ([4ea25db](4ea25db)) * Sync from noir (AztecProtocol/aztec-packages#7862) ([4ea25db](4ea25db)) * Sync from noir (AztecProtocol/aztec-packages#7945) ([4ea25db](4ea25db)) * Sync from noir (AztecProtocol/aztec-packages#7958) ([5c4f19f](5c4f19f)) * Sync from noir (AztecProtocol/aztec-packages#8008) ([5c4f19f](5c4f19f)) * Sync from noir (AztecProtocol/aztec-packages#8093) ([5c4f19f](5c4f19f)) * Sync from noir (AztecProtocol/aztec-packages#8125) ([f0c2686](f0c2686)) * Sync from noir (AztecProtocol/aztec-packages#8237) ([f0c2686](f0c2686)) * Sync from noir (AztecProtocol/aztec-packages#8423) ([3c3ed1e](3c3ed1e)) * Sync from noir (AztecProtocol/aztec-packages#8435) ([3c3ed1e](3c3ed1e)) * Sync from noir (AztecProtocol/aztec-packages#8466) ([3c3ed1e](3c3ed1e)) * Sync from noir (AztecProtocol/aztec-packages#8482) ([d4832ec](d4832ec)) * Sync from noir (AztecProtocol/aztec-packages#8512) ([95e19ab](95e19ab)) * Sync from noir (AztecProtocol/aztec-packages#8526) ([95e19ab](95e19ab)) * TXE nr deployments, dependency cleanup for CLI (AztecProtocol/aztec-packages#7548) ([4ea25db](4ea25db)) * Typing return values of embedded_curve_ops (AztecProtocol/aztec-packages#7413) ([daad75c](daad75c)) * Unify all acir recursion constraints based on RecursionConstraint and proof_type (AztecProtocol/aztec-packages#7993) ([5c4f19f](5c4f19f)) ### Bug Fixes * Add trailing extra arguments for backend in gates_flamegraph (AztecProtocol/aztec-packages#7472) ([daad75c](daad75c)) * **debugger:** Update the debugger to handle the new Brillig debug metadata format ([#5706](#5706)) ([a31f82e](a31f82e)) * Deflatten databus visibilities (AztecProtocol/aztec-packages#7761) ([4ea25db](4ea25db)) * Do not duplicate redundant Brillig debug metadata ([#5696](#5696)) ([e4f7dbe](e4f7dbe)) * Export brillig names in contract functions (AztecProtocol/aztec-packages#8212) ([f0c2686](f0c2686)) * Handle multiple entry points for Brillig call stack resolution after metadata deduplication ([#5788](#5788)) ([38fe9dd](38fe9dd)) * Move BigInt modulus checks to runtime in brillig ([#5374](#5374)) ([741d339](741d339)) * Restrict keccak256_injective test input to 8 bits ([#5977](#5977)) ([a1b1346](a1b1346)) * Revert "feat: Sync from noir (AztecProtocol/aztec-packages#7512)" (AztecProtocol/aztec-packages#7558) ([daad75c](daad75c)) * Runtime brillig bigint id assignment ([#5369](#5369)) ([a8928dd](a8928dd)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem*
Part of general effort to reduce Brillig bytecode sizes
Summary*
We often times (usually when working with nested arrays) run into patterns where we fetch an inner nested array dynamically, array set a different array in the nested array, and the reset the other unchanged nested array at the same index. These array sets can be optimized out.
Here is an example of what happens on master in SSA. Looking at
nested_array_dynamic
with--force-brillig
. Insideb14
you will see the following:After this PR
v118 = array_set v110, index v113, value v114
will simplify tov110
. Ifv114
is unused anywhere else in the function (which in this case it is) it will be removed by DIE.Additional Context
Similarly to
try_optimize_array_get_from_previous_set
we should be able to follow pastarray_sets
when optimizing thesearray_sets
.e.g. if we see a pattern like this:
We know that we performed
array_set
on the samev110
array, however, it was at a different index. So even thoughv120
is different thanv110
we can simplifyv122 = array_set mut v120, index v115, value v116
tov120
.I am working on this in a follow-up, as the simple change in this PR provides decent benefits on its own.
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.