Skip to content

chore(ssa): Do not run Brillig entry point analysis when no globals have been declared#8152

Closed
vezenovm wants to merge 5 commits intomasterfrom
mv/skip-entry-point-dup-when-no-globals
Closed

chore(ssa): Do not run Brillig entry point analysis when no globals have been declared#8152
vezenovm wants to merge 5 commits intomasterfrom
mv/skip-entry-point-dup-when-no-globals

Conversation

@vezenovm
Copy link
Contributor

Description

Problem*

No issue just something I noticed while looking at #8099.

We already skip Brillig entry point specialization if we have a single Brillig entry point (e.g. main is Brillig). We can also avoid specializing Brillig functions for programs that do not use globals at all.

Summary*

Check whether any globals have been declared. If not we can skip the Brillig entry point analysis.

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.

@TomAFrench
Copy link
Member

We've got a panic in brillig_calls_array due to "f4 has multiple entry points"

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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: 86cad8b Previous: 7d8b3bc Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 49 s 40 s 1.23

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

CC: @TomAFrench

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

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

Benchmark suite Current: be1bf92 Previous: 1df587d Ratio
private-kernel-inner 270.8 MB 209.58 MB 1.29
private-kernel-reset 454.35 MB 246.67 MB 1.84
rollup-block-root 7780 MB 1710 MB 4.55
rollup-root 322.46 MB 261.28 MB 1.23

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

CC: @TomAFrench

@vezenovm
Copy link
Contributor Author

We've got a panic in brillig_calls_array due to "f4 has multiple entry points"

Yeah, we then need to handle that we did not specialize entry points during Brillig globals code gen. We also hoist constants during the globals code gen so that complicates things a bit further as well. This is looking to be a bit more trouble than it is worth at the moment, so I am going to close.

@vezenovm vezenovm closed this Apr 22, 2025
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