fix: increment reference counts when deduplicating calls which return arrays by TomAFrench · Pull Request #8757 · noir-lang/noir

TomAFrench · 2025-06-02T15:39:11Z

Description

Problem*

Resolves

Summary*

We would previously deduplicate pure function calls by just replacing the result ids with those of the other function call. This is fine most of the time but when we have a function which returns an array this is roughly equivalent to a make_array instruction so we should have all the same checks.

This PR updates the constant folding pass to treat these two instructions equivalently.

Additional Context

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.

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `8cab58f`	Previous: `a56d36a`	Ratio
`rollup-block-root-empty`	`23.68` s	`18.6` s	`1.27`

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `8cab58f`	Previous: `a56d36a`	Ratio
`rollup-merge`	`0.004` s	`0.003` s	`1.33`

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

TomAFrench · 2025-06-19T11:14:29Z

@aakoshh I'm getting a flake on noir_ast_fuzzer::smoke arb_program_can_be_executed in this PR.

aakoshh · 2025-06-19T11:50:52Z

Captured in #8975

…n calls

github-actions · 2025-06-19T16:26:09Z

Changes to Brillig bytecode sizes

Generated at commit: 69c17d962221b3284909957fe25537b1e4aa150f, compared to commit: a56d36a40a1d72f02892f8dd9360800bcde24b75

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
to_le_bytes_inliner_max	+5 ❌	+4.39%
to_le_bytes_inliner_zero	+5 ❌	+4.39%
to_le_bytes_inliner_min	+5 ❌	+4.07%

Full diff report 👇

Program	Brillig opcodes (+/-)	%
to_le_bytes_inliner_max	119 (+5)	+4.39%
to_le_bytes_inliner_zero	119 (+5)	+4.39%
to_le_bytes_inliner_min	128 (+5)	+4.07%
array_to_slice_inliner_max	509 (+12)	+2.41%
array_to_slice_inliner_zero	682 (+12)	+1.79%
array_to_slice_inliner_min	891 (+12)	+1.37%
aes128_encrypt_inliner_zero	379 (+3)	+0.80%
aes128_encrypt_inliner_min	388 (+3)	+0.78%
aes128_encrypt_inliner_max	406 (+3)	+0.74%
slices_inliner_zero	1,632 (+10)	+0.62%
slices_inliner_max	1,691 (+7)	+0.42%
slices_inliner_min	2,167 (+7)	+0.32%
slice_regex_inliner_zero	1,623 (+3)	+0.19%

github-actions · 2025-06-19T16:26:18Z

Changes to number of Brillig opcodes executed

Generated at commit: 69c17d962221b3284909957fe25537b1e4aa150f, compared to commit: a56d36a40a1d72f02892f8dd9360800bcde24b75

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
array_to_slice_inliner_max	+12 ❌	+1.26%
array_to_slice_inliner_zero	+12 ❌	+0.66%

Full diff report 👇

Program	Brillig opcodes (+/-)	%
array_to_slice_inliner_max	965 (+12)	+1.26%
array_to_slice_inliner_zero	1,826 (+12)	+0.66%
array_to_slice_inliner_min	1,941 (+12)	+0.62%
aes128_encrypt_inliner_max	2,584 (+3)	+0.12%
slices_inliner_zero	2,739 (+3)	+0.11%
aes128_encrypt_inliner_zero	2,873 (+3)	+0.10%
aes128_encrypt_inliner_min	2,886 (+3)	+0.10%
to_le_bytes_inliner_max	1,004 (+1)	+0.10%
to_le_bytes_inliner_zero	1,004 (+1)	+0.10%
to_le_bytes_inliner_min	1,017 (+1)	+0.10%
slice_regex_inliner_zero	3,881 (+3)	+0.08%

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

guipublic

LGTM
Some names could be re-worked as suggested.

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `8cab58f`	Previous: `3838c69`	Ratio
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob`	`187` s	`141` s	`1.33`

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: increment reference counts when deduplicating calls which return arrays (noir-lang/noir#8757) fix: Add a aliased check for last stores (noir-lang/noir#8955) fix(LICM): Consider negative loop bounds before hoisting `div <const> <induction-var>` (noir-lang/noir#8986) fix: check "negate with overflow" in comptime code + allow u1 to be used in comptime code (noir-lang/noir#8969) fix: better package name validation and error messages for nargo new/init (noir-lang/noir#8978) feat(fuzz): Add comptime_vs_brillig_direct target (noir-lang/noir#8924) fix(expand): show references to ModuleDefId recursing on parents (noir-lang/noir#8977) feat(ssa_fuzzer): loops + signed (noir-lang/noir#8881) fix(fuzz): Do not take a mutable reference over immutable vars which contain a mutable ref (noir-lang/noir#8971) fix(fuzz): Avoid negating `i8::MIN` into `i8::MAX+1` which won't compile (noir-lang/noir#8972) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

TomAFrench added 6 commits June 2, 2025 15:32

chore: add test for purity analysis issue

2b67d0f

trigger issue

818bfe5

Merge branch 'master' into tf/add-test-for-purity-issue

62d3382

.

2db88a9

Merge branch 'master' into tf/add-test-for-purity-issue

9501f8a

.

bbc52e2

TomAFrench commented Jun 18, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs Show resolved Hide resolved

github-actions bot reviewed Jun 18, 2025

View reviewed changes

TomAFrench added 2 commits June 18, 2025 17:13

chore: simplify ssa

b90a36e

more simpler

63d0c0f

github-actions bot reviewed Jun 18, 2025

View reviewed changes

TomAFrench added 4 commits June 18, 2025 18:34

.

0e6b55d

.

fe72586

.

7a69323

.

e358c00

fix: increment reference count on arrays returned from cached functio…

02177ce

…n calls

TomAFrench changed the title ~~chore: add regression test for purity analysis issue.~~ fix: increment reference counts when deduplicating calls which return arrays Jun 19, 2025

TomAFrench requested a review from a team June 19, 2025 16:29

TomAFrench marked this pull request as ready for review June 19, 2025 16:29

TomAFrench added 4 commits June 19, 2025 16:44

.

2733deb

.

53ddd22

chore: demonstrate issue still existing

f387fb2

.

de5cd70

guipublic reviewed Jun 20, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs Outdated Show resolved Hide resolved

TomAFrench added 2 commits June 20, 2025 10:33

Merge branch 'master' into tf/add-test-for-purity-issue

edaee44

chore: some quick documentation

a5a124e

TomAFrench added 3 commits June 20, 2025 11:00

chore: loosen restrictions on storing non-array types

ed6cd0f

.

b11a484

Merge branch 'master' into tf/add-test-for-purity-issue

16b72d3

jfecher reviewed Jun 20, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs Outdated Show resolved Hide resolved

guipublic reviewed Jun 20, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs Show resolved Hide resolved

TomAFrench added 2 commits June 20, 2025 17:10

chore: add is_array method

cadbcb8

.

8cab58f

guipublic approved these changes Jun 20, 2025

View reviewed changes

github-actions bot reviewed Jun 20, 2025

View reviewed changes

TomAFrench added this pull request to the merge queue Jun 20, 2025

Merged via the queue into master with commit d7030e8 Jun 20, 2025
118 checks passed

TomAFrench deleted the tf/add-test-for-purity-issue branch June 20, 2025 17:56

noirwhal mentioned this pull request Jun 20, 2025

chore: Release Noir(1.0.0-beta.8) #8892

Merged

AztecBot mentioned this pull request Jun 21, 2025

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: increment reference counts when deduplicating calls which return arrays#8757

fix: increment reference counts when deduplicating calls which return arrays#8757
TomAFrench merged 24 commits intomasterfrom
tf/add-test-for-purity-issue

TomAFrench commented Jun 2, 2025 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot left a comment •

edited

Loading

Uh oh!

github-actions bot left a comment •

edited

Loading

Uh oh!

TomAFrench commented Jun 19, 2025

Uh oh!

aakoshh commented Jun 19, 2025

Uh oh!

github-actions bot commented Jun 19, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jun 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guipublic left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

TomAFrench commented Jun 2, 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!

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

TomAFrench commented Jun 19, 2025

Uh oh!

aakoshh commented Jun 19, 2025

Uh oh!

github-actions bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes to Brillig bytecode sizes

🧾 Summary (10% most significant diffs)

Uh oh!

github-actions bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes to number of Brillig opcodes executed

🧾 Summary (10% most significant diffs)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guipublic left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TomAFrench commented Jun 2, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot commented Jun 19, 2025 •

edited

Loading

github-actions bot commented Jun 19, 2025 •

edited

Loading