Skip to content

fix(ssa): Use Type::element_size instead of Type::flattened_size for optimize_length_one_array_read#10146

Merged
aakoshh merged 5 commits intomasterfrom
af/10141-fix-length-one-array-get
Oct 10, 2025
Merged

fix(ssa): Use Type::element_size instead of Type::flattened_size for optimize_length_one_array_read#10146
aakoshh merged 5 commits intomasterfrom
af/10141-fix-length-one-array-get

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Oct 9, 2025

Description

Problem*

Resolves #10141

Summary*

Fixes the condition for using optimize_length_one_array_read to use element_size instead of flattened_size.

Additional Context

In the failing example we had this SSA:

After Initial SSA:
g0 = u32 0
g1 = u32 1
g2 = u32 3
g3 = make_array [u32 0, u32 1, u32 3] : [u32; 3]
g4 = u1 1
g5 = u1 0
g6 = make_array [u1 1, u1 0] : [u1; 2]
g7 = make_array [g3, g6] : [([u32; 3], [u1; 2]); 1]

acir(inline) fn main f0 {
  b0(v8: u32):
    v10 = call f1() -> [([u64; 0], u16); 1]
    v12 = mul v8, u32 2
    constrain v12 == u32 0, "Index out of bounds"
    v13 = array_get v10, index u32 0 -> [u64; 0]
    v14 = add v12, u32 1
    constrain v14 == u32 0, "Index out of bounds"
    v15 = array_get v10, index u32 0 -> u16
    v16 = cast v15 as Field
    return v16
}
acir(inline) fn foo f1 {
  b0():
    v8 = make_array [] : [u64; 0]
    v10 = make_array [v8, u16 50744] : [([u64; 0], u16); 1]
    return v10
}

The v10 array has 2 elements: [v8, u16 50744], so its .element_size() is 2, but the .flattened_size() is just 1, because a 0 length array like v8 flattens into an empty sequence.

v12 is the base index 0, and v14 is 0+1, which would give us the 1st and 2nd items of the array. However this optimisation falsely concluded that the array is 1 long based on flattened_size, and therefore it can get its only element at index 0, and inserted constraints that both v12 and v14 must be 0, which caused 1) a constraint failure and 2) the casting error, since getting the 0 element returned a [u64; 0] which then went into the cast of the u16 and caused a type error during codegen.

The flattened stuff is a bit confusing; it looks like it's only relevant for ACIR, which I also commented on here. @guipublic and me both got confused for a moment whether v10 is really 1 or 2 long.

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.

@aakoshh aakoshh requested a review from a team October 9, 2025 17:02
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2025

Changes to circuit sizes

Generated at commit: 4d3f773aed14046cdd993b99e7d684482d78cd2d, compared to commit: f19c641a630bdd04cddab0804195fe70197e788b

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
global_nested_array_call_arg_regression -16 ✅ -61.54% -2,773 ✅ -97.09%
nested_array_call_arg_regression -16 ✅ -61.54% -2,773 ✅ -97.09%
nested_array_dynamic_simple -15 ✅ -93.75% -2,770 ✅ -98.05%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_9804 4 (-9) -69.23% 2,802 (-17) -0.60%
global_nested_array_call_arg_regression 10 (-16) -61.54% 83 (-2,773) -97.09%
nested_array_call_arg_regression 10 (-16) -61.54% 83 (-2,773) -97.09%
nested_array_dynamic_simple 1 (-15) -93.75% 55 (-2,770) -98.05%

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: f7f4ee4 Previous: f19c641 Ratio
test_report_zkpassport_noir_rsa_ 2 s 0 s +∞

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

CC: @TomAFrench

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

Some impressive improvements in circuit size now that the optimization is fixed too

@aakoshh aakoshh added this pull request to the merge queue Oct 10, 2025
Merged via the queue into master with commit dbc6cfe Oct 10, 2025
134 checks passed
@aakoshh aakoshh deleted the af/10141-fix-length-one-array-get branch October 10, 2025 10:28
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.

Compile crash: Can only cast numeric types, got Array

3 participants