chore: don't use `set_value_from_id` in `as_slice_length` by asterite · Pull Request #8039 · noir-lang/noir

asterite · 2025-04-11T18:24:21Z

Description

Problem

Slowly trying to migrate away from set_value_from_id

Summary

Three things:

I realized there's never a need to replace values in instruction results: these are always the output of something so they should never be a value previously seen
This PR also removes replace_results as it seems it's no longer needed
I realized that instead of gathering all values to replace, then replacing all of them by traversing all blocks and instructions at the end, we could instead replace values in blocks and instructions the first time we traverse them. This requires using take_instructions() but otherwise it ends up in almost the same code, but one that should be more efficient as we only traverse things once, and we only replace things if we ever find things to replace.

Additional Context

I wonder if it would make sense to have two intrinsics:

as_slice -> (u32, [...])
as_slice_without_length -> [...]

That is, once this optimization runs we also replace the intrinsic with the second one that just returns the slice. Then:

we don't have to worry about not replacing the result with a constant
maybe brillig is a bit faster as it doesn't need to compute the length and put it in a register

Documentation

Check one:

No documentation needed.
Documentation included in this PR.
[For Experimental Features] 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.

…checks`

This reverts commit a31a418.

asterite · 2025-04-14T17:44:50Z

After some refactors this PR is ready for review again.

asterite · 2025-04-15T00:07:03Z

I initially thought this made the bignum tests take a lot longer, though none of the other repos got slower. But now I think this commit in the bignum library, which started using a lot of macros, it the cause of the slowdown.

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: allow keywords in function names in SSA parser (noir-lang/noir#8063) chore: don't use `set_value_from_id` in `as_slice_length` (noir-lang/noir#8039) fix(ssa): Do not use inner else condition when collapsing IfElse based upon the then condition (noir-lang/noir#8040) chore(test): AST generation calibration test (noir-lang/noir#8054) chore(ssa): Remove IfElse pass basic SSA tests (noir-lang/noir#8058) chore(ssa): Additional unrolling and mem2reg passes (noir-lang/noir#8017) chore: bump external pinned commits (noir-lang/noir#8045) chore: bump bb to 0.84.0 (noir-lang/noir#8047) chore: Add `GITHUB_TOKEN` to more workflow steps (noir-lang/noir#8046) feat: skip inverting field elements with values +-1 in ACVM (noir-lang/noir#8049) END_COMMIT_OVERRIDE --------- Co-authored-by: TomAFrench <tom@tomfren.ch> Co-authored-by: AztecBot <tech@aztecprotocol.com>

🤖 I have created a new Aztec Packages release --- ## [0.86.0](v0.85.0...v0.86.0) (2025-04-24) ### ⚠ BREAKING CHANGES * Use combined p2p and http prover coordination ([#13760](#13760)) * use single extended viem client ([#13715](#13715)) * remove slice read from CALL ([#13729](#13729)) ### Features * `#[allow(dead_code)]` (noir-lang/noir#8066) ([366d980](366d980)) * add aztec-up amm test w/ proving & sponsoredfpc ([#13209](#13209)) ([f54c8a1](f54c8a1)) * Add Rollup to registry and produce blocks ([#13556](#13556)) ([9ed10d3](9ed10d3)) * adding devnet tests ([#12808](#12808)) ([7687a1f](7687a1f)) * audit tracking tooling ([#13639](#13639)) ([e50d8e0](e50d8e0)) * **avm:** fast entity indexing without macros ([#13737](#13737)) ([5c8a993](5c8a993)) * **avm:** quick n dirty memory trace ([#13659](#13659)) ([062c6a9](062c6a9)) * **avm:** support col arrays and add keccak cols ([#13711](#13711)) ([9c4345a](9c4345a)) * **avm:** tagged value type in C++ ([#13540](#13540)) ([b30b5b3](b30b5b3)) * avoid unnecessary zero check in brillig overflow check (noir-lang/noir#8109) ([366d980](366d980)) * **contracts:** static + snapshotted validator set ([#13046](#13046)) ([90033f2](90033f2)) * enable mempool limiter on devnet ([#13722](#13722)) ([36fb0a1](36fb0a1)) * exec opcode spec table ([#13594](#13594)) ([d4d503a](d4d503a)) * **experimental:** Implement separate `-Zownership` analysis for ownership pass (noir-lang/noir#7861) ([366d980](366d980)) * expose mempool limit through helm chart ([#13616](#13616)) ([822e8d9](822e8d9)) * Garaga UltraStarknet[Zk]Honk flavours ([#11489](#11489)) ([6bc34a1](6bc34a1)) * IPA claim as public component ([#13429](#13429)) ([1ce3c43](1ce3c43)) * no longer recompute vk's in CIVC proofs ([#13590](#13590)) ([66a61ba](66a61ba)) * **p2p:** add private peers ([#12585](#12585)) ([4264c8c](4264c8c)) * **p2p:** optional P2P_BROADCAST_PORT ([#13525](#13525)) ([f709fab](f709fab)) * Port callstack resolution from aztec to noirjs (noir-lang/noir#7642) ([366d980](366d980)) * Refactor IPA claim handling in acir format to support them for AVM ([#13547](#13547)) ([df73c05](df73c05)) * remove slice read from CALL ([#13729](#13729)) ([7e7eb85](7e7eb85)) * replace field divisions by constants with multiplication by inv… (noir-lang/noir#8053) ([366d980](366d980)) * report world state size on disk ([#13706](#13706)) ([0dd8a7e](0dd8a7e)) * skip inverting field elements with values +-1 in ACVM (noir-lang/noir#8049) ([44c1347](44c1347)) * SMT Verificaiton Module: Data Structures ([#13658](#13658)) ([2de3bc5](2de3bc5)) * SMT verification module updates ([#13551](#13551)) ([f02123d](f02123d)) * ssa fuzzer (noir-lang/noir#7641) ([366d980](366d980)) * Tighter timing on ACVM ([#13743](#13743)) ([45cd39b](45cd39b)) * track rewards and slots ([#13546](#13546)) ([489c6cf](489c6cf)) * Use combined p2p and http prover coordination ([#13760](#13760)) ([4d04e62](4d04e62)) * VK generation test for HonkRecursionConstraint ([#13637](#13637)) ([71e81ce](71e81ce)) ### Bug Fixes * **acir:** Check whether opcodes were laid down for non-equality check before fetching payload locations (noir-lang/noir#8133) ([366d980](366d980)) * add proper handling for `u128` in comptime interpreter (noir-lang/noir#8079) ([366d980](366d980)) * Add version to gossipsub protocol ([#13567](#13567)) ([8ce4829](8ce4829)) * allow keywords in function names in SSA parser (noir-lang/noir#8063) ([44c1347](44c1347)) * Allow more slack in AST calibration for CI (noir-lang/noir#8076) ([366d980](366d980)) * amm bot ([#13553](#13553)) ([6b93832](6b93832)), closes [#13544](#13544) * **avm:** cpp addressing ([#13652](#13652)) ([9d941b6](9d941b6)) * blob sink waits for reg address when needed ([#13628](#13628)) ([486a42a](486a42a)) * **brillig:** SliceRefCount reads from the appropriate pointer (noir-lang/noir#8148) ([366d980](366d980)) * Check against multiple aws profiles in build instance credentails ([#13421](#13421)) ([71a408a](71a408a)) * dependency cycles in public simulator - part 0 (sim -> context) ([#13678](#13678)) ([e00089f](e00089f)) * dependency cycles in public simulator - part 1 (errors/revertReason) ([#13679](#13679)) ([c75e365](c75e365)) * dependency cycles in public simulator part 2 (serializable bytecode) ([#13680](#13680)) ([5d3e24c](5d3e24c)) * discv5 test failure ([#13653](#13653)) ([58c143b](58c143b)) * **docs:** Make viewing (outdated) protocol specs work locally ([#13534](#13534)) ([694537d](694537d)) * Increase timeout for p2p integration test ([#13720](#13720)) ([c244b2e](c244b2e)) * mac bb --version ([#13690](#13690)) ([9f8b64f](9f8b64f)) * make translator use ultra rather than eccvm ops ([#13489](#13489)) ([53c070d](53c070d)) * more worker fixes ([#13759](#13759)) ([080485a](080485a)) * move helm lint to build ([#13532](#13532)) ([7768ff8](7768ff8)) * native ivc benches not publishing ([#13665](#13665)) ([7ab712d](7ab712d)) * no exports of simulator should depend on jest-mock-extended ([#13694](#13694)) ([65a9f33](65a9f33)) * only clear trailing zeros on logs ([#13727](#13727)) ([e392d7c](e392d7c)) * **p2p:** better batch connection sampling ([#13674](#13674)) ([9c1d9f1](9c1d9f1)) * **p2p:** reqresp types + batch request tx pool filtering ([#13666](#13666)) ([8e2a3c9](8e2a3c9)) * **parser:** error on missing let semicolon in trait (and others) (noir-lang/noir#8101) ([366d980](366d980)) * pass along coinbase ([#13560](#13560)) ([593f810](593f810)), closes [#13643](#13643) * prover-node start ([#13627](#13627)) ([5d87f7a](5d87f7a)) * quote numbers ([#13654](#13654)) ([f0d1ec1](f0d1ec1)) * remove all txs from a failed epoch ([#13771](#13771)) ([abc4623](abc4623)) * remove insecure dummy round derivation from sumcheck and shplemini ([#13488](#13488)) ([9a3bb46](9a3bb46)) * replace values in data_bus too (noir-lang/noir#8086) ([366d980](366d980)) * retry deploy npm ([#13691](#13691)) ([76cab3f](76cab3f)) * run yarn ([#13713](#13713)) ([0e51259](0e51259)) * squash nightly foundry warning ([#13640](#13640)) ([e21f9fd](e21f9fd)) * SSA pass print filter to include the count (noir-lang/noir#8074) ([366d980](366d980)) * **ssa:** Do not inline simple recursive functions (noir-lang/noir#8127) ([366d980](366d980)) * **ssa:** Do not unroll loop with break (noir-lang/noir#8090) ([366d980](366d980)) * **ssa:** Do not use inner else condition when collapsing IfElse based upon the then condition (noir-lang/noir#8040) ([44c1347](44c1347)) * **ssa:** fix possibility to `Field % Field` operaions in Brillig from SSA (noir-lang/noir#8105) ([366d980](366d980)) * **ssa:** Loop range with u1 (noir-lang/noir#8131) ([366d980](366d980)) * update metric name to avoid conflicts ([#13629](#13629)) ([70c58ab](70c58ab)), closes [#13626](#13626) * wasm memory benchmark ([#13573](#13573)) ([9a73c4a](9a73c4a)) * wrapping mul support for u128 (noir-lang/noir#7941) ([366d980](366d980)) ### Miscellaneous * Accept optional test path for emitting integration tests (noir-lang/noir#8062) ([366d980](366d980)) * Add `GITHUB_TOKEN` to more workflow steps (noir-lang/noir#8046) ([44c1347](44c1347)) * add a benchmark for opcodes which need a batchable inversion (noir-lang/noir#8110) ([366d980](366d980)) * add hacky faster bootstrap for bb-centric e2e flow ([#13587](#13587)) ([498d433](498d433)) * address some visibility warnings ([#13728](#13728)) ([35dec90](35dec90)) * **avm:** less codegen for lookups ([#13741](#13741)) ([ec468d9](ec468d9)) * **avm:** move interaction jobs to trace builders ([#13621](#13621)) ([caac1c9](caac1c9)) * bb.js non-inlined web workers ([#13736](#13736)) ([0e604a1](0e604a1)) * **bb:** debugging helpers ([#13584](#13584)) ([0ebb29e](0ebb29e)) * **bb:** Make goblin a proper source module ([#13580](#13580)) ([3048a14](3048a14)) * bespoke export for client native prover / PXE server store lazy load ([#13783](#13783)) ([640dd08](640dd08)) * Blob and archiver syncing improvements ([#13542](#13542)) ([629aa2a](629aa2a)), closes [#13530](#13530) * blob archive client factory code ([#13648](#13648)) ([7b16791](7b16791)) * bug list ([#13773](#13773)) ([c6d4fb8](c6d4fb8)) * bump axois ([#13453](#13453)) ([1c20920](1c20920)) * bump bb to 0.84.0 (noir-lang/noir#8047) ([44c1347](44c1347)) * bump bignum timeout (noir-lang/noir#8080) ([366d980](366d980)) * bump external pinned commits (noir-lang/noir#8045) ([44c1347](44c1347)) * **ci:** run non dependent bootstrap build steps in parallel ([#13454](#13454)) ([b17e18d](b17e18d)) * cleanup aztec up amm test ([#13754](#13754)) ([3d92afe](3d92afe)) * Cleanup scripts in package jsons in yarn-project ([#13527](#13527)) ([a296945](a296945)) * **contracts:** core/staking -> core/slashing ([#13748](#13748)) ([5d93a0e](5d93a0e)) * create module for array handling in acirgen (noir-lang/noir#8119) ([366d980](366d980)) * dashboards use ${data_source} variable ([#13613](#13613)) ([45533d9](45533d9)) * delete honk_recursion for building ACIR ([#13664](#13664)) ([ac95729](ac95729)) * delete Ultra Vanilla CIVC ([#13669](#13669)) ([ae578a2](ae578a2)) * delete zeromorph ([#13667](#13667)) ([c8acae0](c8acae0)) * Disable blobscan by default ([#13742](#13742)) ([c55088e](c55088e)) * **docs:** `aztec` and `aztec-wallet` cli reference updates ([#13692](#13692)) ([d11d154](d11d154)) * **docs:** add 0.85.0 docs ([#13578](#13578)) ([cf07358](cf07358)) * **docs:** add AND/XOR bug to bug list ([#13784](#13784)) ([797ae6d](797ae6d)) * **docs:** Add architecture docs (noir-lang/noir#7992) ([366d980](366d980)) * **docs:** adding redirects for node guide ([#13768](#13768)) ([e96e5bf](e96e5bf)) * **docs:** bugfix + deploy on master ([#13717](#13717)) ([010975a](010975a)) * **docs:** Fix some stuff on fees, rm confusing storage example ([#13501](#13501)) ([c00b20b](c00b20b)) * **docs:** Sequencer and Prover Guide ([#13433](#13433)) ([55b9a2b](55b9a2b)) * **docs:** update bb commands to match 0.84.0 (noir-lang/noir#8050) ([366d980](366d980)) * **docs:** Update docs bootrap to to include release ([#13615](#13615)) ([2154d5e](2154d5e)) * don't use `set_from_value_id` in `remove_if_else` (noir-lang/noir#8070) ([366d980](366d980)) * don't use `set_value_from_id` in `as_slice_length` (noir-lang/noir#8039) ([44c1347](44c1347)) * don't use `set_value_from_id` in `constant_folding` (noir-lang/noir#8091) ([366d980](366d980)) * don't use `set_value_from_id` in `loop_invariant` (noir-lang/noir#8085) ([366d980](366d980)) * don't use `set_value_from_id` in `remove_bit_shifts` (noir-lang/noir#8071) ([366d980](366d980)) * don't use `set_value_from_id` in `simplify_cfg` (noir-lang/noir#8072) ([366d980](366d980)) * **experimental:** Function::simple_optimization for SSA optimizations (noir-lang/noir#8102) ([366d980](366d980)) * Fetch rollup address using version as index ([#13620](#13620)) ([60e73f9](60e73f9)) * Fix license files ([#13695](#13695)) ([9931fb8](9931fb8)) * mempool limit ([#13735](#13735)) ([def1287](def1287)) * mint block rewards for 200K blocks at deployment ([#13537](#13537)) ([1a3a326](1a3a326)) * move check_circuit functionality from `TranslatorCircuitBuilder` into a `TranslatorCircuitChecker` ([#13761](#13761)) ([e4e7fca](e4e7fca)) * noir test flake addition ([#13663](#13663)) ([c302fdd](c302fdd)) * **noir-contracts:** update readme ([#13563](#13563)) ([42dfbbf](42dfbbf)) * **optimization:** Enable experimental ownership clone scheme by default (noir-lang/noir#8097) ([366d980](366d980)) * parse nop in SSA parser (noir-lang/noir#8141) ([366d980](366d980)) * ping luke instead of cody on bb benchmark regressions ([#13718](#13718)) ([2802f5a](2802f5a)) * playground improvements ([#13588](#13588)) ([4c3453b](4c3453b)) * **playground:** bump vite ([#13750](#13750)) ([7e72420](7e72420)) * Print `unsafe { ... }` around calls to Brillig from ACIR in AST (noir-lang/noir#8077) ([366d980](366d980)) * **public/avm:** from hints to the end of the world ([#13459](#13459)) ([a89de5d](a89de5d)) * redo typo PR by dizer-ti ([#13757](#13757)) ([075d3a8](075d3a8)) * release playground on s3. ([#13582](#13582)) ([2f9458a](2f9458a)) * **release:** give aztec release image sensible default entrypoint ([#13517](#13517)) ([a864c06](a864c06)) * remove circuit simulator ([#13689](#13689)) ([a00566c](a00566c)) * Remove code of the fake AVM recursive verifier ([#13614](#13614)) ([048dfed](048dfed)) * remove msm sorter ([#13668](#13668)) ([ff29d86](ff29d86)) * remove old terraform configs ([#13716](#13716)) ([49ac1db](49ac1db)), closes [#13651](#13651) * remove omit param from serialize derivation ([#13703](#13703)) ([6af5943](6af5943)) * remove pcs vk from vk ([#13638](#13638)) ([0e941ef](0e941ef)) * remove try_merge_only_changed_indices (noir-lang/noir#8142) ([366d980](366d980)) * replace relative paths to noir-protocol-circuits ([78c87b5](78c87b5)) * replace relative paths to noir-protocol-circuits ([4e53cd1](4e53cd1)) * replace relative paths to noir-protocol-circuits ([3a33c47](3a33c47)) * replace relative paths to noir-protocol-circuits ([eaaa85b](eaaa85b)) * replace relative paths to noir-protocol-circuits ([648a4fe](648a4fe)) * replace relative paths to noir-protocol-circuits ([b3f4745](b3f4745)) * replace relative paths to noir-protocol-circuits ([269b316](269b316)) * replace relative paths to noir-protocol-circuits ([1330efe](1330efe)) * replace relative paths to noir-protocol-circuits ([a739284](a739284)) * Sanity check block number from archiver before returning it ([#13631](#13631)) ([e459b2a](e459b2a)) * simpler `make_mutable` in `array_set` optimization (noir-lang/noir#8106) ([366d980](366d980)) * **spartan:** bump default kind resources ([#13747](#13747)) ([8ca828f](8ca828f)) * **ssa:** Additional unrolling and mem2reg passes (noir-lang/noir#8017) ([44c1347](44c1347)) * **ssa:** Remove IfElse pass basic SSA tests (noir-lang/noir#8058) ([44c1347](44c1347)) * **ssa:** Test terminator value constant folding and resolve cache for data bus (noir-lang/noir#8132) ([366d980](366d980)) * starknet feature flag ([#13681](#13681)) ([2b6c627](2b6c627)) * stop prover node swallowing start prover job errors ([#13676](#13676)) ([90c2b7b](90c2b7b)) * **test:** AST generation calibration test (noir-lang/noir#8054) ([44c1347](44c1347)) * update ACVM doc (noir-lang/noir#8004) ([366d980](366d980)) * Update docs snippet ([#13739](#13739)) ([7fb333a](7fb333a)) * update staking asset handler address ([#13566](#13566)) ([daa09f2](daa09f2)) * Updated contract addresses for alpha-testnet ([#13585](#13585)) ([9f0ff4a](9f0ff4a)) * Use chain monitor to sync system time in p2p tests ([#13632](#13632)) ([abbad4c](abbad4c)) * use public component key for pairing inputs ([#13705](#13705)) ([467166f](467166f)) * use PublicComponentKeys ([#13686](#13686)) ([68e4b6f](68e4b6f)) * use single extended viem client ([#13715](#13715)) ([cf637dc](cf637dc)) * validate block header hash in circuits ([#13094](#13094)) ([f19c182](f19c182)) * wasm proof verifying with native bb ([#13499](#13499)) ([1ad9249](1ad9249)) ### Documentation * fees tutorial ([#13357](#13357)) ([c580657](c580657)) * move snippets all versions ([#13650](#13650)) ([a2f5553](a2f5553)) * Update glossary & snippets ([#13661](#13661)) ([d55816d](d55816d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: AztecBot <tech@aztecprotocol.com>

asterite added 10 commits April 11, 2025 13:55

chore: don't use set_value_from_id in `remove_truncate_after_range_…

6ad5af8

…checks`

Use FxHashMap

3a136fd

Replace in all blocks

0e4b912

Make sure values are replaced across blocks, and fix optimization

ad73aa4

Add a comment

b49a9bb

chore: don't use set_value_from_id in as_slice_length

1eea960

Undo visibility change

b990e29

Don't set constants in result positions

3f81295

Bring back replace_result

a31a418

Revert "Bring back replace_result"

3067725

This reverts commit a31a418.

asterite mentioned this pull request Apr 11, 2025

chore: don't use set_value_from_id in remove_truncate_after_range_checks #8037

Merged

5 tasks

Base automatically changed from ab/replace_values_in_remove_truncate_after_range_check to master April 11, 2025 19:58

Merge branch 'master' into ab/replace_values_in_as_slice_length

3286492

asterite requested a review from a team April 11, 2025 20:21

asterite added the Blocked PR is blocked on an issue label Apr 11, 2025

No need to replace instruction results

ebb6e7a

asterite removed the Blocked PR is blocked on an issue label Apr 12, 2025

asterite added 3 commits April 14, 2025 10:02

Do as_slice_length optimization in one pass

7a86973

Implement remove_truncate_after_range_check in one go

cd294fd

Merge branch 'master' into ab/replace_values_in_as_slice_length

7fe4b1d

asterite mentioned this pull request Apr 14, 2025

chore: don't use set_value_from_id in simplify_cfg #8059

Closed

5 tasks

asterite added 3 commits April 14, 2025 14:10

Recursively solve values

a33eaa8

clippy

34f8e08

Merge branch 'master' into ab/replace_values_in_as_slice_length

6c2fe6d

This was referenced Apr 14, 2025

chore: don't use set_value_from_id in remove_if_else #8041

Closed

chore: don't use set_value_from_id in remove_bit_shifts #8056

Closed

jfecher approved these changes Apr 14, 2025

View reviewed changes

asterite added this pull request to the merge queue Apr 14, 2025

github-merge-queue bot pushed a commit that referenced this pull request Apr 14, 2025

chore: don't use set_value_from_id in as_slice_length (#8039)

b0fd93b

Merged via the queue into master with commit e3314c9 Apr 14, 2025
113 checks passed

asterite deleted the ab/replace_values_in_as_slice_length branch April 14, 2025 21:47

AztecBot mentioned this pull request Apr 15, 2025

chore: Bump Noir reference AztecProtocol/aztec-packages#13554

Merged

AztecBot mentioned this pull request Apr 15, 2025

chore(master): release 0.86.0 AztecProtocol/aztec-packages#13579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: don't use `set_value_from_id` in `as_slice_length`#8039

chore: don't use `set_value_from_id` in `as_slice_length`#8039
asterite merged 18 commits intomasterfrom
ab/replace_values_in_as_slice_length

asterite commented Apr 11, 2025 •

edited

Loading

Uh oh!

asterite commented Apr 14, 2025

Uh oh!

Uh oh!

asterite commented Apr 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

asterite commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Summary

Additional Context

Documentation

PR Checklist

Uh oh!

asterite commented Apr 14, 2025

Uh oh!

Uh oh!

asterite commented Apr 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

asterite commented Apr 11, 2025 •

edited

Loading