Skip to content

fix: Do not carry over #[fold] to unconstrained functions during monomorphization#10155

Merged
aakoshh merged 7 commits intomasterfrom
af/10153-fold-becomes-brillig
Oct 10, 2025
Merged

fix: Do not carry over #[fold] to unconstrained functions during monomorphization#10155
aakoshh merged 7 commits intomasterfrom
af/10153-fold-becomes-brillig

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Oct 10, 2025

Description

Problem*

Resolves #10153

Summary*

Check whether the primary inline function attribute is compatible with the function we are creating; if not, use the default inline type.

Additional Context

During monomorphization when we call an ACIR function from an unconstrained one, it changes the runtime and creates an unconstrained version. During this the doppelgänger function variant inherited all the function attributes from the original, including ACIR-only attributes that the front end would have rejected if it saw them.

Another instance where this slipped in was the use function pointers: to be able to pass them on without knowing whether they will be called from constrained or unconstrained context, we create a both versions of them.

Here's the monomorphized AST showing the integration test no longer containing the #[fold] for the unconstrained version of foo:

fn main$f0() -> pub Field {
    let _f$l0 = (foo$f1, foo$f2);
    {
        bar$f3()
    }
}
#[fold]
fn foo$f1() -> Field {
    0
}
unconstrained fn foo$f2() -> Field {
    0
}
unconstrained fn bar$f3() -> Field {
    foo$f2()
}

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 changed the title fix: Do not inherit #[fold] onto unconstrained functions during monomorphization fix: Do not carry over #[fold] to unconstrained functions during monomorphization Oct 10, 2025
@aakoshh aakoshh requested a review from a team October 10, 2025 11:19
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

LGTM after removing unnecessary Prover.toml

@aakoshh aakoshh enabled auto-merge October 10, 2025 11:23
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: bff82c7 Previous: cedc8ec Ratio
test_report_zkpassport_noir-ecdsa_ 3 s 2 s 1.50

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: bff82c7 Previous: cedc8ec Ratio
rollup-block-root-single-tx 0.003 s 0.002 s 1.50

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

CC: @TomAFrench

@TomAFrench
Copy link
Member

Hmm, that's an odd error.

@aakoshh
Copy link
Contributor Author

aakoshh commented Oct 10, 2025

Yes, apparently Lints disallow no_predicates on Brillig functions, but in fact they are required for inlining to get rid of some calls in these tests that must not survive until Brillig-gen

@aakoshh
Copy link
Contributor Author

aakoshh commented Oct 10, 2025

I'm checking why it wasn't just simplified out on its own.

@aakoshh
Copy link
Contributor Author

aakoshh commented Oct 10, 2025

The reason we need to keep the no_predicates is because derive_pedersen_generators cannot reach Brillig-gen, but they can only be simplified out if its arguments are constants. In the pedersen_check we have the following SSA:

//After Flattening (1) (step 22):
g0 = Field 340282366920938463463374607431768211456

brillig(inline) predicate_pure fn main f0 {
  ...
  b3():
    ...
    v28 = make_array [v1, v2] : [Field; 2]
    v30 = call f1(v28, u32 0) -> Field
    ...
    return
}
brillig(no_predicates) predicate_pure fn pedersen_hash_with_separator f1 {
  b0(v1: [Field; 2], v2: u32):
    v5 = make_array [Field 0, Field 0, Field 0, Field 0, Field 0, Field 0] : [(Field, Field); 3]
    v6 = allocate -> &mut [(Field, Field); 3]
    store v5 at v6
    v8 = make_array [Field 0, Field 0, u1 1, Field 0, Field 0, u1 1, Field 0, Field 0, u1 1] : [(Field, Field, u1); 3]
    v9 = allocate -> &mut [(Field, Field, u1); 3]
    store v8 at v9
    v25 = make_array b"DEFAULT_DOMAIN_SEPARATOR"
    inc_rc v25
    v27 = call derive_pedersen_generators(v25, v2) -> [(Field, Field, u1); 2]
    jmp b1(u32 0)
  ...
}

So, even though mem2reg, which comes next, does try to simplify this, it can't because v2 is a parameter to pedersen_hash_with_separator.

Once we get to the "Inlining" pass, it gets inlined into main and disappears:

After Inlining (2) (step 24):
g0 = Field 340282366920938463463374607431768211456

brillig(inline) predicate_pure fn main f0 {
  ...
  b3():
    ...
    v29 = make_array [v1, v2] : [Field; 2]
    v30 = make_array [Field 0, Field 0, Field 0, Field 0, Field 0, Field 0] : [(Field, Field); 3]
    v31 = allocate -> &mut [(Field, Field); 3]
    store v30 at v31
    v32 = make_array [Field 0, Field 0, u1 1, Field 0, Field 0, u1 1, Field 0, Field 0, u1 1] : [(Field, Field, u1); 3]
    v33 = allocate -> &mut [(Field, Field, u1); 3]
    store v32 at v33
    v49 = make_array b"DEFAULT_DOMAIN_SEPARATOR"
    inc_rc v49
    v50 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0, Field 2393473289045184898987089634332637236754766663897650125720167164137088869378, Field -7135402912423807765050323395026152633898511180575289670895350565966806597339, u1 0] : [(Field, Field, u1); 2]
    jmp b4(u32 0)

@aakoshh aakoshh force-pushed the af/10153-fold-becomes-brillig branch from 67a7d04 to 1c1ad93 Compare October 10, 2025 13:40
@aakoshh aakoshh added this pull request to the merge queue Oct 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 10, 2025
@aakoshh aakoshh added this pull request to the merge queue Oct 10, 2025
Merged via the queue into master with commit 11c175d Oct 10, 2025
132 checks passed
@aakoshh aakoshh deleted the af/10153-fold-becomes-brillig branch October 10, 2025 15:24
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
@Savio-Sou Savio-Sou added this to the Group 0 Audited - External 2 milestone Dec 2, 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.

Crash: Brillig call to #[fold] ACIR function

3 participants