Skip to content

chore(ACIRgen): always compute array offset#10099

Merged
TomAFrench merged 2 commits intomasterfrom
ab/acir_gen_always_compute_offset
Oct 8, 2025
Merged

chore(ACIRgen): always compute array offset#10099
TomAFrench merged 2 commits intomasterfrom
ab/acir_gen_always_compute_offset

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented Oct 7, 2025

Description

Problem

Resolves #10085

Summary

This is just trying this out. But the explanation to this is: I found it odd that compute_offset would sometimes return an offset, but sometimes not. Why not? In which cases we wouldn't be able to compute an offset? An what happens in those cases? (maybe we are lacking tests?)

The problem was that compute_offset searched a type, in the array item types, that was equal to the type of the instruction result. This works fine for ArrayGet, but for ArraySet the type of the result is always the same array and it will never match the type of an item type. For ArraySet we must search the type of the store value. Then we can always compute an offset.

Now, I don't know if computing an offset is always desirable, and there's a chance that this will make performance worse, but I feel this way is more consistent. So I'm mainly opening this PR to further explore this issue.

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.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2025

Changes to circuit sizes

Generated at commit: 5149549892afd8e3ac6359118e49a6dce73ae58e, compared to commit: 75c7d69cf14633c6ce8de84e1e204bdf8ecc33bd

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
hashmap -1 ✅ -0.00% -1 ✅ -0.00%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
hashmap 31,849 (-1) -0.00% 93,825 (-1) -0.00%

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: 1aba0bf Previous: 75c7d69 Ratio
test_report_zkpassport_noir_rsa_ 2 s 1 s 2

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 Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 1aba0bf Previous: 75c7d69 Ratio
rollup-checkpoint-merge 0.004 s 0.003 s 1.33

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

CC: @TomAFrench

@asterite
Copy link
Collaborator Author

asterite commented Oct 7, 2025

image

Well, not a huge win 😅

But I do feel the code ends up more consistent, and there's also less code (some chunks are gone now).

@asterite asterite requested a review from a team October 7, 2025 14:26
@TomAFrench TomAFrench added this pull request to the merge queue Oct 8, 2025
Merged via the queue into master with commit a5e29fe Oct 8, 2025
134 checks passed
@TomAFrench TomAFrench deleted the ab/acir_gen_always_compute_offset branch October 8, 2025 11:54
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
TomAFrench added a commit that referenced this pull request Oct 13, 2025
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: add unit tests for brillig-gen (noir-lang/noir#10130)
chore(ACIR): remove non-ACIR intrinsics during simplification (noir-lang/noir#10145)
fix: Do not carry over `#[fold]` to unconstrained functions during monomorphization (noir-lang/noir#10155)
chore(SSA): avoid consuming self when returning Arc (noir-lang/noir#10147)
fix(ssa): Use `Type::element_size` instead of `Type::flattened_size` for `optimize_length_one_array_read` (noir-lang/noir#10146)
fix(ssa): SSA Interpreter handle overflow by promoting to Field (noir-lang/noir#10097)
chore: Try to optimize compilation memory (noir-lang/noir#10113)
chore(ACIRgen): smaller AcirDynamicArray value_types (noir-lang/noir#10128)
chore(brillig_vm): Re-org integration tests and add a couple more (noir-lang/noir#10129)
chore: unhide `inliner-aggressiveness` option (noir-lang/noir#10137)
chore(brillig_vm): Expand arithmetic int ops tests and add field ops tests (noir-lang/noir#10101)
chore(ACIR): don't override output count in black box function (noir-lang/noir#10123)
chore(test): Add `interpret_execution_failure` tests (noir-lang/noir#9912)
fix(ACIR): correctly display the zero expression (noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs` (noir-lang/noir#10055)
chore: add brillig_call submodule (noir-lang/noir#10108)
chore(ACIRgen): always compute array offset (noir-lang/noir#10099)
chore: More BTreeSet avoidance (noir-lang/noir#10107)
END_COMMIT_OVERRIDE

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: add unit tests for brillig-gen (noir-lang/noir#10130)
chore(ACIR): remove non-ACIR intrinsics during simplification (noir-lang/noir#10145)
fix: Do not carry over `#[fold]` to unconstrained functions during monomorphization (noir-lang/noir#10155)
chore(SSA): avoid consuming self when returning Arc (noir-lang/noir#10147)
fix(ssa): Use `Type::element_size` instead of `Type::flattened_size` for `optimize_length_one_array_read` (noir-lang/noir#10146)
fix(ssa): SSA Interpreter handle overflow by promoting to Field (noir-lang/noir#10097)
chore: Try to optimize compilation memory (noir-lang/noir#10113)
chore(ACIRgen): smaller AcirDynamicArray value_types (noir-lang/noir#10128)
chore(brillig_vm): Re-org integration tests and add a couple more (noir-lang/noir#10129)
chore: unhide `inliner-aggressiveness` option (noir-lang/noir#10137)
chore(brillig_vm): Expand arithmetic int ops tests and add field ops tests (noir-lang/noir#10101)
chore(ACIR): don't override output count in black box function (noir-lang/noir#10123)
chore(test): Add `interpret_execution_failure` tests (noir-lang/noir#9912)
fix(ACIR): correctly display the zero expression (noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs` (noir-lang/noir#10055)
chore: add brillig_call submodule (noir-lang/noir#10108)
chore(ACIRgen): always compute array offset (noir-lang/noir#10099)
chore: More BTreeSet avoidance (noir-lang/noir#10107)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: add unit tests for brillig-gen
(noir-lang/noir#10130)
chore(ACIR): remove non-ACIR intrinsics during simplification
(noir-lang/noir#10145)
fix: Do not carry over `#[fold]` to unconstrained functions during
monomorphization (noir-lang/noir#10155)
chore(SSA): avoid consuming self when returning Arc
(noir-lang/noir#10147)
fix(ssa): Use `Type::element_size` instead of `Type::flattened_size` for
`optimize_length_one_array_read`
(noir-lang/noir#10146)
fix(ssa): SSA Interpreter handle overflow by promoting to Field
(noir-lang/noir#10097)
chore: Try to optimize compilation memory
(noir-lang/noir#10113)
chore(ACIRgen): smaller AcirDynamicArray value_types
(noir-lang/noir#10128)
chore(brillig_vm): Re-org integration tests and add a couple more
(noir-lang/noir#10129)
chore: unhide `inliner-aggressiveness` option
(noir-lang/noir#10137)
chore(brillig_vm): Expand arithmetic int ops tests and add field ops
tests (noir-lang/noir#10101)
chore(ACIR): don't override output count in black box function
(noir-lang/noir#10123)
chore(test): Add `interpret_execution_failure` tests
(noir-lang/noir#9912)
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: add unit tests for brillig-gen
(noir-lang/noir#10130)
chore(ACIR): remove non-ACIR intrinsics during simplification
(noir-lang/noir#10145)
fix: Do not carry over `#[fold]` to unconstrained functions during
monomorphization (noir-lang/noir#10155)
chore(SSA): avoid consuming self when returning Arc
(noir-lang/noir#10147)
fix(ssa): Use `Type::element_size` instead of `Type::flattened_size` for
`optimize_length_one_array_read`
(noir-lang/noir#10146)
fix(ssa): SSA Interpreter handle overflow by promoting to Field
(noir-lang/noir#10097)
chore: Try to optimize compilation memory
(noir-lang/noir#10113)
chore(ACIRgen): smaller AcirDynamicArray value_types
(noir-lang/noir#10128)
chore(brillig_vm): Re-org integration tests and add a couple more
(noir-lang/noir#10129)
chore: unhide `inliner-aggressiveness` option
(noir-lang/noir#10137)
chore(brillig_vm): Expand arithmetic int ops tests and add field ops
tests (noir-lang/noir#10101)
chore(ACIR): don't override output count in black box function
(noir-lang/noir#10123)
chore(test): Add `interpret_execution_failure` tests
(noir-lang/noir#9912)
fix(ACIR): correctly display the zero expression
(noir-lang/noir#10124)
chore: typos and some refactors in `acvm/src/pwg/mod.rs`
(noir-lang/noir#10055)
chore: add brillig_call submodule
(noir-lang/noir#10108)
chore(ACIRgen): always compute array offset
(noir-lang/noir#10099)
chore: More BTreeSet avoidance
(noir-lang/noir#10107)
END_COMMIT_OVERRIDE
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.

Revisit offset variable in ACIR-gen

2 participants