fix(defunctionalize): Create a placeholder function for first-class function calls with no variants by vezenovm · Pull Request #8697 · noir-lang/noir

vezenovm · 2025-05-27T20:29:22Z

Description

Problem*

Resolves last bug from #8672 with empty arrays.

Alternative to #8677

Summary*

For function calls with no variants we no longer skip updating the calls. We generate a dummy function so that there is still a concrete call to a real function rather than an SSA value ID. This was something I was debating when originally fixing the compiler panic for not having any function variants #8573. At the time it did not cause issues to simply not update those variant-less calls. To be safe, we can now add a dummy function that allows us to continue compilation and not break the semantics of other passes. This includes generating the parameters and return values for the function (e.g., what if inlining attempts to inline the dummy function). This also makes the defunctionalization pass more accurate, as no matter what every function used as a value will be transformed into a concrete call, even those that come from a broken IR (e.g. OOB access on a zero-length array).

The other solution aside #8677 would be to ban accessing zero-length arrays. However, this feels a bit more like a hack and this PR's solution helps prevent similar bugs in the future.

Additional Context

#8677 and its parent #8672

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.

github-actions

ACVM Benchmarks

Details

Benchmark suite	Current: `6954395`	Previous: `8aa2267`	Ratio
`purely_sequential_opcodes`	`256950` ns/iter (`± 309`)	`249494` ns/iter (`± 419`)	`1.03`
`perfectly_parallel_opcodes`	`227359` ns/iter (`± 4660`)	`218261` ns/iter (`± 3475`)	`1.04`
`perfectly_parallel_batch_inversion_opcodes`	`2786904` ns/iter (`± 1491`)	`2777239` ns/iter (`± 2030`)	`1.00`

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

github-actions

Execution Time

Details

Benchmark suite	Current: `6954395`	Previous: `8aa2267`	Ratio
`private-kernel-inner`	`0.027` s	`0.027` s	`1`
`private-kernel-reset`	`0.161` s	`0.162` s	`0.99`
`private-kernel-tail`	`0.012` s	`0.011` s	`1.09`
`rollup-base-private`	`0.286` s	`0.287` s	`1.00`
`rollup-base-public`	`0.187` s	`0.183` s	`1.02`
`rollup-block-root`	`13.1` s	`12.9` s	`1.02`
`rollup-merge`	`0.004` s	`0.004` s	`1`
`rollup-root`	`0.005` s	`0.006` s	`0.83`
`semaphore-depth-10`	`0.019` s	`0.019` s	`1`

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

github-actions

Compilation Time

Details

Benchmark suite	Current: `6954395`	Previous: `8aa2267`	Ratio
`private-kernel-inner`	`2.338` s	`2.28` s	`1.03`
`private-kernel-reset`	`7.346` s	`7.644` s	`0.96`
`private-kernel-tail`	`1.09` s	`1.052` s	`1.04`
`rollup-base-private`	`15.26` s	`15.22` s	`1.00`
`rollup-base-public`	`13.46` s	`13.72` s	`0.98`
`rollup-block-root-empty`	`18.82` s	`18.28` s	`1.03`
`rollup-block-root-single-tx`	`175` s	`181` s	`0.97`
`rollup-block-root`	`184` s	`179` s	`1.03`
`rollup-merge`	`1.196` s	`1.2` s	`1.00`
`rollup-root`	`1.278` s	`1.312` s	`0.97`
`semaphore-depth-10`	`0.758` s	`0.747` s	`1.01`

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

github-actions

Artifact Size

Details

Benchmark suite	Current: `6954395`	Previous: `8aa2267`	Ratio
`private-kernel-inner`	`1135.3` KB	`1135.3` KB	`1`
`private-kernel-reset`	`2072.5` KB	`2072.5` KB	`1`
`private-kernel-tail`	`589.1` KB	`589.1` KB	`1`
`rollup-base-private`	`4933.9` KB	`4933.9` KB	`1`
`rollup-base-public`	`3974.6` KB	`3974.6` KB	`1`
`rollup-block-root-empty`	`3876.9` KB	`3876.9` KB	`1`
`rollup-block-root-single-tx`	`32734.1` KB	`32734.1` KB	`1`
`rollup-block-root`	`32764` KB	`32764` KB	`1`
`rollup-merge`	`185.8` KB	`185.8` KB	`1`
`rollup-root`	`410.3` KB	`410.3` KB	`1`
`semaphore-depth-10`	`636.4` KB	`636.4` KB	`1`

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

vezenovm · 2025-05-27T21:03:32Z

Actually I'm going to try one more alternative, where we just insert an Instruction::Noop for these calls.

github-actions

Test Suite Duration

Details

Benchmark suite	Current: `6954395`	Previous: `3838c69`	Ratio
`test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr`	`75` s	`75` s	`1`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts`	`123` s	`127` s	`0.97`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob`	`147` s	`141` s	`1.04`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib`	`207` s	`204` s	`1.01`
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib`	`552` s
`test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types`	`71` s	`73` s	`0.97`
`test_report_noir-lang_noir-bignum_`	`385` s
`test_report_noir-lang_noir_bigcurve_`	`236` s
`test_report_noir-lang_sha512_`	`16` s	`15` s	`1.07`
`test_report_zkpassport_noir-ecdsa_`	`105` s	`115` s	`0.91`
`test_report_zkpassport_noir_rsa_`	`2` s	`2` s	`1`

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

github-actions

Compilation Memory

Details

Benchmark suite	Current: `6954395`	Previous: `8aa2267`	Ratio
`private-kernel-inner`	`293.13` MB	`293.09` MB	`1.00`
`private-kernel-reset`	`534.18` MB	`534.18` MB	`1`
`private-kernel-tail`	`191.71` MB	`191.71` MB	`1`
`rollup-base-private`	`1400` MB	`1400` MB	`1`
`rollup-base-public`	`1530` MB	`1530` MB	`1`
`rollup-block-root-empty`	`1090` MB	`1090` MB	`1`
`rollup-block-root-single-tx`	`9410` MB	`9410` MB	`1`
`rollup-block-root`	`9420` MB	`9420` MB	`1`
`rollup-merge`	`344.62` MB	`344.6` MB	`1.00`
`rollup-root`	`354.6` MB	`354.61` MB	`1.00`
`semaphore_depth_10`	`106.45` MB	`106.45` MB	`1`

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

github-actions

Execution Memory

Details

Benchmark suite	Current: `6954395`	Previous: `8aa2267`	Ratio
`private-kernel-inner`	`202.19` MB	`202.19` MB	`1`
`private-kernel-reset`	`225.94` MB	`225.94` MB	`1`
`private-kernel-tail`	`177.48` MB	`177.48` MB	`1`
`rollup-base-private`	`507.26` MB	`507.26` MB	`1`
`rollup-base-public`	`440.45` MB	`440.45` MB	`1`
`rollup-block-root`	`1510` MB	`1510` MB	`1`
`rollup-merge`	`329.54` MB	`329.54` MB	`1`
`rollup-root`	`331.64` MB	`331.64` MB	`1`
`semaphore_depth_10`	`71.02` MB	`71.02` MB	`1`

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

vezenovm · 2025-05-27T21:28:04Z

Actually I'm going to try one more alternative, where we just insert an Instruction::Noop for these calls.

This is actually more pain than it is worth as we are simply replacing the call instruction directly and don't have a function inserter in the defunctionalization pass for mapping instruction results if we were to simply remove the call. It is easiest to maintain the structure of the call and have the dummy function match what its apply function would have been.

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

…o mv/empty-array-of-lambdas-2

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

vezenovm · 2025-06-18T15:56:11Z

This PR is ready for review again.

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

asterite

Looks great!

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

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: `b8e6528`	Previous: `71ab596`	Ratio
`rollup-merge`	`0.004` s	`0.003` s	`1.33`

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

CC: @TomAFrench

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

Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix(defunctionalize): Create a placeholder function for first-class function calls with no variants (noir-lang/noir#8697) fix(mem2reg): Keep store when any aliased reference is kept (noir-lang/noir#8960) fix(parser): let `as` have a lower precedence (noir-lang/noir#8956) fix: Match against all Value recursive types when checking for a function/closure in a global (noir-lang/noir#8967) fix(formatter): reset indetnation after group changed it (noir-lang/noir#8966) chore(validation): Ban function pointers in SSA globals (noir-lang/noir#8947) chore: address various clippy issues (noir-lang/noir#8942) chore(fuzz): Consider `RangeCheckFailed` equivalent to `ConstantDoesNotFitType` if the value matches (noir-lang/noir#8958) chore(fuzz): Use `nextest` to run nightly fuzz test (noir-lang/noir#8962) chore: minor code quality issues (noir-lang/noir#8961) END_COMMIT_OVERRIDE --------- Co-authored-by: AztecBot <tech@aztecprotocol.com> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

vezenovm added 2 commits May 27, 2025 20:21

gen dummy func for no variants

6170755

imrpvoe comments

2c83863

vezenovm changed the title ~~fix(defunctionalize): Create a placeholder function for no first-class function calls with no variants~~ fix(defunctionalize): Create a placeholder function for first-class function calls with no variants May 27, 2025

vezenovm mentioned this pull request May 27, 2025

fix(ssa): Empty array of higher order functions #8677

Closed

5 tasks

Merge branch 'master' into mv/empty-array-of-lambdas-2

2229efd

vezenovm requested review from a team and jfecher May 27, 2025 20:32

remove unreachable funcs after defunctionalize

8e5fbc2

vezenovm added the bench-show Display benchmark results on PR label May 27, 2025

change type of field

0560ad1

github-actions bot reviewed May 27, 2025

View reviewed changes

use use apply functions for dummy

2b9c759

vezenovm commented May 27, 2025

View reviewed changes

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

Update compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs

8a6ae34

vezenovm commented May 27, 2025

View reviewed changes

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

vezenovm added 3 commits May 27, 2025 17:29

Update compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs

563c8a2

bump timeout

48f600f

Merge remote-tracking branch 'origin/mv/empty-array-of-lambdas-2' int…

4c37881

…o mv/empty-array-of-lambdas-2

vezenovm commented May 28, 2025

View reviewed changes

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

vezenovm added 3 commits June 17, 2025 21:05

merge conflicts

d2741ef

fix dummy placeholder function return values for a well formed IR

96e649d

dont need additional remove unreachable

95fbc92

vezenovm added 2 commits June 18, 2025 15:55

remove top line import

f21f861

Merge branch 'master' into mv/empty-array-of-lambdas-2

d31f0dd

asterite reviewed Jun 18, 2025

View reviewed changes

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

asterite approved these changes Jun 18, 2025

View reviewed changes

vezenovm commented Jun 18, 2025

View reviewed changes

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

Update compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs

6c7b80e

vezenovm enabled auto-merge June 18, 2025 17:12

github-actions bot reviewed Jun 18, 2025

View reviewed changes

vezenovm commented Jun 18, 2025

View reviewed changes

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

vezenovm added 4 commits June 18, 2025 13:40

Update compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs

39d86ac

Merge branch 'master' into mv/empty-array-of-lambdas-2

85583bf

don't store when returning dummy ref

b8e6528

Merge branch 'master' into mv/empty-array-of-lambdas-2

6954395

vezenovm added this pull request to the merge queue Jun 18, 2025

Merged via the queue into master with commit e96b40c Jun 18, 2025
118 checks passed

vezenovm deleted the mv/empty-array-of-lambdas-2 branch June 18, 2025 21:49

noirwhal mentioned this pull request Jun 18, 2025

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

Merged

AztecBot mentioned this pull request Jun 19, 2025

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

Merged

Conversation

vezenovm commented May 27, 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!

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

ACVM Benchmarks

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

Execution Time

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

Compilation Time

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

Artifact Size

Uh oh!

vezenovm commented May 27, 2025

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

Test Suite Duration

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

Compilation Memory

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

Execution Memory

Uh oh!

vezenovm commented May 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vezenovm commented Jun 18, 2025

Uh oh!

Uh oh!

asterite left a comment

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vezenovm commented May 27, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading