feat(brillig): Centralize memory layout policy and reorganize memory regions#9985
feat(brillig): Centralize memory layout policy and reorganize memory regions#9985
Conversation
|
Converting to draft due to fuzz test errors. |
…rame, expand the memory layout tests to show how entry point differs
I found the bug. This is ready for review again. |
…into mv/brillig-memory-layout-reorg
There was a problem hiding this comment.
⚠️ 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: 9409f87 | Previous: 8d4f14d | Ratio |
|---|---|---|---|
sha512-100-bytes |
0.076 s |
0.054 s |
1.41 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ 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: 9409f87 | Previous: 8d4f14d | Ratio |
|---|---|---|---|
test_report_zkpassport_noir-ecdsa_ |
3 s |
1 s |
3 |
test_report_zkpassport_noir_rsa_ |
2 s |
1 s |
2 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ 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: c4a8dc1 | Previous: 5e75f23 | Ratio |
|---|---|---|---|
rollup-checkpoint-root |
243 s |
190 s |
1.28 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
Shouldn't be an issue. |
TomAFrench
left a comment
There was a problem hiding this comment.
Looks good, some small nits
Just for additional clarity we currently set the heap to start after the entry point memory region |
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: Remove unnecessary allocation in `expr_with` (noir-lang/noir#10103) chore(ACIR): inline `maybe_eq_predicate` (noir-lang/noir#10095) chore: use `u64` over `Field` in template program (noir-lang/noir#10096) chore: disallow slice arguments to blackbox functions (noir-lang/noir#10090) chore(brillig_vm): Separate fuzzing module (noir-lang/noir#10091) chore: ConstrainNotEqual requires acir predicate (noir-lang/noir#10062) chore: bump version of bb used in tests (noir-lang/noir#10093) chore: wrapping arithmetic tests (noir-lang/noir#9714) chore(brillig_vm): Foreign call module and test re-org (noir-lang/noir#10089) chore: add extra constraint folding pass (noir-lang/noir#9766) chore: refactor brillig_blocks (noir-lang/noir#10088) chore: add regression test for #9986 (noir-lang/noir#10087) chore: Release Noir(1.0.0-beta.14) (noir-lang/noir#9942) fix(tag_attr): keep whitespace tokens when parsing (noir-lang/noir#9981) fix: hoist and then deduplicate (noir-lang/noir#10047) chore: typos and some refactors in `acvm/src/pwg` (noir-lang/noir#10086) chore: add in hack for `public_dispatch` (noir-lang/noir#10084) fix(ssa): Avoid going through `i128` when casting signed to `u128` (noir-lang/noir#10045) chore: avoid zero bits range-checks (noir-lang/noir#10083) chore: bump external pinned commits (noir-lang/noir#10082) fix(stdlib): Only compute the garbage `embedded_curve_result` result if we know we will need it (noir-lang/noir#10077) fix(ssa): Keep defaults for values returned in the databus (noir-lang/noir#10042) chore: remove unused predicate from mem-op solver (noir-lang/noir#10079) chore(ACIR): snapshot tests for each instruction (noir-lang/noir#10071) fix: remove generic length from ECDSA message hash in stdlib (noir-lang/noir#10043) chore: validate that no jumps to function entry block exist (noir-lang/noir#10076) feat(brillig): Centralize memory layout policy and reorganize memory regions (noir-lang/noir#9985) chore(ci): fix permissions about publishing rustdoc (noir-lang/noir#10075) chore(ACVM): use Vec instead of Hash for memory blocks (noir-lang/noir#10072) feat(ssa): `constant_folding` with loop (noir-lang/noir#10019) chore: take truncate into account for bit size (noir-lang/noir#10059) chore: update check for `u128` overflow in `check_u128_mul_overflow` (noir-lang/noir#9998) chore: update check for field overflow in `check_u128_mul_overflow` (noir-lang/noir#9968) chore(ACIR): binary instructions snapshots (noir-lang/noir#10054) chore(acir): SliceRemove refactor (noir-lang/noir#10058) fix(fuzzer): Mark DivisionByZero with different types as equivalent (noir-lang/noir#10066) chore(fuzz): Remove `is_frontend_friendly` from the AST fuzzer (noir-lang/noir#10046) chore: use new ACIR syntax in docs, and some tests (noir-lang/noir#10057) fix(ssa): SSA interpreter to use the 2nd arg in `slice_refcount` (noir-lang/noir#10034) fix(ssa): SSA interpreter to return 0 for `Intrinsic::*RefCount` when constrained (noir-lang/noir#10033) chore(ssa_fuzzer): fix array get/set (noir-lang/noir#10031) fix(acir): Extend slice on dynamic insertion and compilation panic when flattening (noir-lang/noir#10051) chore(ACIR): extract convert_constrain_error helper (noir-lang/noir#10050) chore(ACIR): expand signed lt, div and mod in SSA (noir-lang/noir#10036) chore(ACIR): more consistent syntax and with less noise (noir-lang/noir#10014) chore(acir): Code gen tests for slice intrinsics (noir-lang/noir#10017) feat: parse and display SSA databus (noir-lang/noir#9991) fix(ssa): Handle partially removed `ArrayGet` groups of complex type during OOB checks (noir-lang/noir#10027) chore(acir): binary operations always have the same operand types (noir-lang/noir#10028) feat: Add Module::parent and Module::child_modules (noir-lang/noir#10005) chore: green light for ACVM optimisation (noir-lang/noir#10002) chore: unit test for brillig solver (greenlight ACVM execution) (noir-lang/noir#9967) chore(acir): avoid duplication when invoking brillig stdlib call (noir-lang/noir#10025) chore: Use 8 partitions for rust tests (noir-lang/noir#10026) chore: green light for ACVM execution audit (noir-lang/noir#9982) chore: greenlight for ACVM execution (PWG) (noir-lang/noir#9961) feat: optimize out noop casts on constants (noir-lang/noir#10024) fix(mem2reg): consider call return aliases (noir-lang/noir#10016) chore: bump external pinned commits (noir-lang/noir#10022) fix(parser): enforce left brace after match expression (noir-lang/noir#10018) chore(acir): Intrinsics and slice_ops modules as well as slice_ops doc comments (noir-lang/noir#10012) fix: signed division by -1 can overflow (noir-lang/noir#9976) chore(ci): fix external checks (noir-lang/noir#10009) chore(ci): add provenance attestations to npm packages (noir-lang/noir#10011) chore(ACIR): show all expressions as polynomials (noir-lang/noir#10007) chore: remove unused feature flag (noir-lang/noir#9993) chore(ci): fix docs breaking JS releases (noir-lang/noir#10010) chore(ssa_fuzzer): add external coverage registration (noir-lang/noir#9974) END_COMMIT_OVERRIDE Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: Remove unnecessary allocation in `expr_with` (noir-lang/noir#10103) chore(ACIR): inline `maybe_eq_predicate` (noir-lang/noir#10095) chore: use `u64` over `Field` in template program (noir-lang/noir#10096) chore: disallow slice arguments to blackbox functions (noir-lang/noir#10090) chore(brillig_vm): Separate fuzzing module (noir-lang/noir#10091) chore: ConstrainNotEqual requires acir predicate (noir-lang/noir#10062) chore: bump version of bb used in tests (noir-lang/noir#10093) chore: wrapping arithmetic tests (noir-lang/noir#9714) chore(brillig_vm): Foreign call module and test re-org (noir-lang/noir#10089) chore: add extra constraint folding pass (noir-lang/noir#9766) chore: refactor brillig_blocks (noir-lang/noir#10088) chore: add regression test for #9986 (noir-lang/noir#10087) chore: Release Noir(1.0.0-beta.14) (noir-lang/noir#9942) fix(tag_attr): keep whitespace tokens when parsing (noir-lang/noir#9981) fix: hoist and then deduplicate (noir-lang/noir#10047) chore: typos and some refactors in `acvm/src/pwg` (noir-lang/noir#10086) chore: add in hack for `public_dispatch` (noir-lang/noir#10084) fix(ssa): Avoid going through `i128` when casting signed to `u128` (noir-lang/noir#10045) chore: avoid zero bits range-checks (noir-lang/noir#10083) chore: bump external pinned commits (noir-lang/noir#10082) fix(stdlib): Only compute the garbage `embedded_curve_result` result if we know we will need it (noir-lang/noir#10077) fix(ssa): Keep defaults for values returned in the databus (noir-lang/noir#10042) chore: remove unused predicate from mem-op solver (noir-lang/noir#10079) chore(ACIR): snapshot tests for each instruction (noir-lang/noir#10071) fix: remove generic length from ECDSA message hash in stdlib (noir-lang/noir#10043) chore: validate that no jumps to function entry block exist (noir-lang/noir#10076) feat(brillig): Centralize memory layout policy and reorganize memory regions (noir-lang/noir#9985) chore(ci): fix permissions about publishing rustdoc (noir-lang/noir#10075) chore(ACVM): use Vec instead of Hash for memory blocks (noir-lang/noir#10072) feat(ssa): `constant_folding` with loop (noir-lang/noir#10019) chore: take truncate into account for bit size (noir-lang/noir#10059) chore: update check for `u128` overflow in `check_u128_mul_overflow` (noir-lang/noir#9998) chore: update check for field overflow in `check_u128_mul_overflow` (noir-lang/noir#9968) chore(ACIR): binary instructions snapshots (noir-lang/noir#10054) chore(acir): SliceRemove refactor (noir-lang/noir#10058) fix(fuzzer): Mark DivisionByZero with different types as equivalent (noir-lang/noir#10066) chore(fuzz): Remove `is_frontend_friendly` from the AST fuzzer (noir-lang/noir#10046) chore: use new ACIR syntax in docs, and some tests (noir-lang/noir#10057) fix(ssa): SSA interpreter to use the 2nd arg in `slice_refcount` (noir-lang/noir#10034) fix(ssa): SSA interpreter to return 0 for `Intrinsic::*RefCount` when constrained (noir-lang/noir#10033) chore(ssa_fuzzer): fix array get/set (noir-lang/noir#10031) fix(acir): Extend slice on dynamic insertion and compilation panic when flattening (noir-lang/noir#10051) chore(ACIR): extract convert_constrain_error helper (noir-lang/noir#10050) chore(ACIR): expand signed lt, div and mod in SSA (noir-lang/noir#10036) chore(ACIR): more consistent syntax and with less noise (noir-lang/noir#10014) chore(acir): Code gen tests for slice intrinsics (noir-lang/noir#10017) feat: parse and display SSA databus (noir-lang/noir#9991) fix(ssa): Handle partially removed `ArrayGet` groups of complex type during OOB checks (noir-lang/noir#10027) chore(acir): binary operations always have the same operand types (noir-lang/noir#10028) feat: Add Module::parent and Module::child_modules (noir-lang/noir#10005) chore: green light for ACVM optimisation (noir-lang/noir#10002) chore: unit test for brillig solver (greenlight ACVM execution) (noir-lang/noir#9967) chore(acir): avoid duplication when invoking brillig stdlib call (noir-lang/noir#10025) chore: Use 8 partitions for rust tests (noir-lang/noir#10026) chore: green light for ACVM execution audit (noir-lang/noir#9982) chore: greenlight for ACVM execution (PWG) (noir-lang/noir#9961) feat: optimize out noop casts on constants (noir-lang/noir#10024) fix(mem2reg): consider call return aliases (noir-lang/noir#10016) chore: bump external pinned commits (noir-lang/noir#10022) fix(parser): enforce left brace after match expression (noir-lang/noir#10018) chore(acir): Intrinsics and slice_ops modules as well as slice_ops doc comments (noir-lang/noir#10012) fix: signed division by -1 can overflow (noir-lang/noir#9976) chore(ci): fix external checks (noir-lang/noir#10009) chore(ci): add provenance attestations to npm packages (noir-lang/noir#10011) chore(ACIR): show all expressions as polynomials (noir-lang/noir#10007) chore: remove unused feature flag (noir-lang/noir#9993) chore(ci): fix docs breaking JS releases (noir-lang/noir#10010) chore(ssa_fuzzer): add external coverage registration (noir-lang/noir#9974) END_COMMIT_OVERRIDE
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE chore: Remove unnecessary allocation in `expr_with` (noir-lang/noir#10103) chore(ACIR): inline `maybe_eq_predicate` (noir-lang/noir#10095) chore: use `u64` over `Field` in template program (noir-lang/noir#10096) chore: disallow slice arguments to blackbox functions (noir-lang/noir#10090) chore(brillig_vm): Separate fuzzing module (noir-lang/noir#10091) chore: ConstrainNotEqual requires acir predicate (noir-lang/noir#10062) chore: bump version of bb used in tests (noir-lang/noir#10093) chore: wrapping arithmetic tests (noir-lang/noir#9714) chore(brillig_vm): Foreign call module and test re-org (noir-lang/noir#10089) chore: add extra constraint folding pass (noir-lang/noir#9766) chore: refactor brillig_blocks (noir-lang/noir#10088) chore: add regression test for #9986 (noir-lang/noir#10087) chore: Release Noir(1.0.0-beta.14) (noir-lang/noir#9942) fix(tag_attr): keep whitespace tokens when parsing (noir-lang/noir#9981) fix: hoist and then deduplicate (noir-lang/noir#10047) chore: typos and some refactors in `acvm/src/pwg` (noir-lang/noir#10086) chore: add in hack for `public_dispatch` (noir-lang/noir#10084) fix(ssa): Avoid going through `i128` when casting signed to `u128` (noir-lang/noir#10045) chore: avoid zero bits range-checks (noir-lang/noir#10083) chore: bump external pinned commits (noir-lang/noir#10082) fix(stdlib): Only compute the garbage `embedded_curve_result` result if we know we will need it (noir-lang/noir#10077) fix(ssa): Keep defaults for values returned in the databus (noir-lang/noir#10042) chore: remove unused predicate from mem-op solver (noir-lang/noir#10079) chore(ACIR): snapshot tests for each instruction (noir-lang/noir#10071) fix: remove generic length from ECDSA message hash in stdlib (noir-lang/noir#10043) chore: validate that no jumps to function entry block exist (noir-lang/noir#10076) feat(brillig): Centralize memory layout policy and reorganize memory regions (noir-lang/noir#9985) chore(ci): fix permissions about publishing rustdoc (noir-lang/noir#10075) chore(ACVM): use Vec instead of Hash for memory blocks (noir-lang/noir#10072) feat(ssa): `constant_folding` with loop (noir-lang/noir#10019) chore: take truncate into account for bit size (noir-lang/noir#10059) chore: update check for `u128` overflow in `check_u128_mul_overflow` (noir-lang/noir#9998) chore: update check for field overflow in `check_u128_mul_overflow` (noir-lang/noir#9968) chore(ACIR): binary instructions snapshots (noir-lang/noir#10054) chore(acir): SliceRemove refactor (noir-lang/noir#10058) fix(fuzzer): Mark DivisionByZero with different types as equivalent (noir-lang/noir#10066) chore(fuzz): Remove `is_frontend_friendly` from the AST fuzzer (noir-lang/noir#10046) chore: use new ACIR syntax in docs, and some tests (noir-lang/noir#10057) fix(ssa): SSA interpreter to use the 2nd arg in `slice_refcount` (noir-lang/noir#10034) fix(ssa): SSA interpreter to return 0 for `Intrinsic::*RefCount` when constrained (noir-lang/noir#10033) chore(ssa_fuzzer): fix array get/set (noir-lang/noir#10031) fix(acir): Extend slice on dynamic insertion and compilation panic when flattening (noir-lang/noir#10051) chore(ACIR): extract convert_constrain_error helper (noir-lang/noir#10050) chore(ACIR): expand signed lt, div and mod in SSA (noir-lang/noir#10036) chore(ACIR): more consistent syntax and with less noise (noir-lang/noir#10014) chore(acir): Code gen tests for slice intrinsics (noir-lang/noir#10017) feat: parse and display SSA databus (noir-lang/noir#9991) fix(ssa): Handle partially removed `ArrayGet` groups of complex type during OOB checks (noir-lang/noir#10027) chore(acir): binary operations always have the same operand types (noir-lang/noir#10028) feat: Add Module::parent and Module::child_modules (noir-lang/noir#10005) chore: green light for ACVM optimisation (noir-lang/noir#10002) chore: unit test for brillig solver (greenlight ACVM execution) (noir-lang/noir#9967) chore(acir): avoid duplication when invoking brillig stdlib call (noir-lang/noir#10025) chore: Use 8 partitions for rust tests (noir-lang/noir#10026) chore: green light for ACVM execution audit (noir-lang/noir#9982) chore: greenlight for ACVM execution (PWG) (noir-lang/noir#9961) feat: optimize out noop casts on constants (noir-lang/noir#10024) fix(mem2reg): consider call return aliases (noir-lang/noir#10016) chore: bump external pinned commits (noir-lang/noir#10022) fix(parser): enforce left brace after match expression (noir-lang/noir#10018) chore(acir): Intrinsics and slice_ops modules as well as slice_ops doc comments (noir-lang/noir#10012) fix: signed division by -1 can overflow (noir-lang/noir#9976) chore(ci): fix external checks (noir-lang/noir#10009) chore(ci): add provenance attestations to npm packages (noir-lang/noir#10011) chore(ACIR): show all expressions as polynomials (noir-lang/noir#10007) chore: remove unused feature flag (noir-lang/noir#9993) chore(ci): fix docs breaking JS releases (noir-lang/noir#10010) chore(ssa_fuzzer): add external coverage registration (noir-lang/noir#9974) END_COMMIT_OVERRIDE Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Description
Problem*
Resolves #9815
The issue highlighted in #9815 may not be a correctness bug per se, however, it does open up the possibility of breaking reproducible builds. The crux of the issue is that our byte code is dependent on the
MAX_STACK_SIZEconstant. This means that if the constant were to change that the same program would produce different byte code. If the constants were to ever change we would lose reproducible builds across those versions of the compiler. In general our memory layout is hard to reason about and needs to be made clearer.Summary*
I ended up labeling this PR a feat as it does more than simply re-organize the memory regions.
Changes:
MAX_STACK_SIZE,MAX_STACK_FRAME_SIZE,MAX_SCRATCH_SPACE) withLayoutConfig. Memory regions are now configurable per program compilation.{reserved} {scratch} {globals} {entry_point (calldata + return data)} {stack} {heap}
Stack,ScratchSpace,GlobalSpace) now compute start/end addresses fromLayoutConfigrather than the hardcoded constantsLayoutConfigas to centralize the memory layout policyCheckMaxStackDepthprocedure has the accurate constant to check the stack pointer againstLayoutConfigthis test would not have been possible without config attributes or some other gross method. The tests also show how the Brillig artifacts are not affected at all by the memory layout and only the final entry point artifact is affected. In the previous memory layout every Brillig artifact was different when the constants were changed.Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.