-
Notifications
You must be signed in to change notification settings - Fork 238
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
fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit #7242
Conversation
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.
⚠️ 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: f6bf769 | Previous: 15bbaa8 | Ratio |
---|---|---|---|
noir-lang_schnorr_ |
1 s |
0 s |
+∞ |
noir-lang_noir_check_shuffle_ |
1 s |
0 s |
+∞ |
noir-lang_mimc_ |
1 s |
0 s |
+∞ |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
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.
This seems much simpler than going off of brillig instructions
…ir#7257) feat(experimental): Implement enum tag constants (noir-lang/noir#7183) fix(unrolling): Fetch original bytecode size from the original function (noir-lang/noir#7253) fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit (noir-lang/noir#7242) feat: Sync from aztec-packages (noir-lang/noir#7241) chore: bump gates diff (noir-lang/noir#7245) feat: simplify subtraction from self to return zero (noir-lang/noir#7189) feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186) fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239) feat: Allow resolved types in constructors (noir-lang/noir#7223) chore: clarify to_radix docs examples (noir-lang/noir#7230) chore: fix struct example (noir-lang/noir#7198) feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197) chore: start tracking time to run critical library tests (noir-lang/noir#7221) chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222) fix(brillig): Globals entry point reachability analysis (noir-lang/noir#7188) chore: update docs to use devcontainer feature (noir-lang/noir#7206) chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209) chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211) chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203) chore: build docs in the merge queue (noir-lang/noir#7218) fix: correct reversed callstacks (noir-lang/noir#7212) chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210) feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
feat(experimental): Implement enum tag constants (noir-lang/noir#7183) fix(unrolling): Fetch original bytecode size from the original function (noir-lang/noir#7253) fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit (noir-lang/noir#7242) feat: Sync from aztec-packages (noir-lang/noir#7241) chore: bump gates diff (noir-lang/noir#7245) feat: simplify subtraction from self to return zero (noir-lang/noir#7189) feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186) fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239) feat: Allow resolved types in constructors (noir-lang/noir#7223) chore: clarify to_radix docs examples (noir-lang/noir#7230) chore: fix struct example (noir-lang/noir#7198) feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197) chore: start tracking time to run critical library tests (noir-lang/noir#7221) chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222) fix(brillig): Globals entry point reachability analysis (noir-lang/noir#7188) chore: update docs to use devcontainer feature (noir-lang/noir#7206) chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209) chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211) chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203) chore: build docs in the merge queue (noir-lang/noir#7218) fix: correct reversed callstacks (noir-lang/noir#7212) chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210) feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
…ir#7257) feat(experimental): Implement enum tag constants (noir-lang/noir#7183) fix(unrolling): Fetch original bytecode size from the original function (noir-lang/noir#7253) fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit (noir-lang/noir#7242) feat: Sync from aztec-packages (noir-lang/noir#7241) chore: bump gates diff (noir-lang/noir#7245) feat: simplify subtraction from self to return zero (noir-lang/noir#7189) feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186) fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239) feat: Allow resolved types in constructors (noir-lang/noir#7223) chore: clarify to_radix docs examples (noir-lang/noir#7230) chore: fix struct example (noir-lang/noir#7198) feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197) chore: start tracking time to run critical library tests (noir-lang/noir#7221) chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222) fix(brillig): Globals entry point reachability analysis (noir-lang/noir#7188) chore: update docs to use devcontainer feature (noir-lang/noir#7206) chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209) chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211) chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203) chore: build docs in the merge queue (noir-lang/noir#7218) fix: correct reversed callstacks (noir-lang/noir#7212) chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210) feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
feat(experimental): Implement enum tag constants (noir-lang/noir#7183) fix(unrolling): Fetch original bytecode size from the original function (noir-lang/noir#7253) fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit (noir-lang/noir#7242) feat: Sync from aztec-packages (noir-lang/noir#7241) chore: bump gates diff (noir-lang/noir#7245) feat: simplify subtraction from self to return zero (noir-lang/noir#7189) feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186) fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239) feat: Allow resolved types in constructors (noir-lang/noir#7223) chore: clarify to_radix docs examples (noir-lang/noir#7230) chore: fix struct example (noir-lang/noir#7198) feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197) chore: start tracking time to run critical library tests (noir-lang/noir#7221) chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222) fix(brillig): Globals entry point reachability analysis (noir-lang/noir#7188) chore: update docs to use devcontainer feature (noir-lang/noir#7206) chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209) chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211) chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203) chore: build docs in the merge queue (noir-lang/noir#7218) fix: correct reversed callstacks (noir-lang/noir#7212) chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210) feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
Description
Problem*
Pulls a change out of #7236 that was causing panics with less aggressive inliner settings. Comment for reference: #7236 (comment).
I labelled this PR a fix rather than a chore as it does prevent possible panics.
Summary*
After performing an unroll in the Brillig runtime we have a setting that lets us set an upper bound on the bytecode size increase percentage. However, we have certain intrinsics (e.g. DerivePedersenGenerators) that we expect to entirely compile-time and fully resolved by Brillig gen.
Another possible fix would be to set any method which calls
derive_generators
(such aspedersen_hash_with_separator
) as#[inline_always]
. However, we still do not have a guarantee that theseparator
has been resolved to a constant. Let's say we knowderive_generators
is called bypedersen_hash_with_separator
. Even ifpedersen_hash_with_separator
is always inlined into its parent we cannot guarantee that the input for the separator comes from the parent's function parameter. This issue will exist with any other intrinsics we expect to be fully resolved by compile-time.By using the number of SSA instructions we can also avoid the extra compilation to Brillig as well as having to expose extra Brillig gen logic to the SSA passes. This gives us a better separation of concerns.
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.