Skip to content

fix: more SSA interpreter fixes#8904

Merged
asterite merged 19 commits intomasterfrom
ab/ssa-interpreter-fixes-2
Jun 13, 2025
Merged

fix: more SSA interpreter fixes#8904
asterite merged 19 commits intomasterfrom
ab/ssa-interpreter-fixes-2

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented Jun 12, 2025

Description

Problem

Resolves #8894

Summary

There are several commits here explaining what got fixed.

The changes in brillig codes, etc., is because a u32 was changed to Field when doing codegen for a format string (this is fixing a bug).

I also added a few things to the intepreter:

  • when defining a value (the define method), we now always check that the type of the value we assign to is the same as the type of the value on the right-hand side. This caught a ton of bugs.
  • I renamed --print-definitions to --trace because just printing definitions wasn't clear enough: if a function was called the same value IDs might show up on the screen, but they are for a different function. So now we get to see when functions are entered and exited, and also block jumps.

Here's a sample trace:

acir(inline) fn main f0 {
  b0(v0: Field):
    v3 = call f1(v0) -> Field
    v5 = eq v3, Field 0
    jmpif v5 then: b1, else: b2
  b1():
    v9 = add v3, Field 1
    jmp b3(v9)
  b2():
    v7 = add v3, Field 10
    jmp b3(v7)
  b3(v1: Field):
    return v1
}
acir(inline) fn foo f1 {
  b0(v0: Field):
    v2 = add v0, Field 1
    v4 = add v0, Field 2
    v5 = add v2, v4
    return v5
}


enter function f0 (main)
v0 = Field 10

enter function f1 (foo)
v0 = Field 10
v2 = Field 11
v4 = Field 12
v5 = Field 23
return Field 23
exit function f1 (foo)

back in function f0 (main)
v3 = Field 23
v5 = u1 false
jump to b2
v7 = Field 33
jump to b3
v1 = Field 33
return Field 33
exit function f0 (main)

--- Interpreter result after Initial SSA:
Ok([Numeric(Field(33))])
---

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 Jun 12, 2025

Changes to number of Brillig opcodes executed

Generated at commit: b8c323e1b3c41947ccf9d2aed9a045b6a7a63671, compared to commit: 7902a216bd9a052ec57aaad6071bbaa779d4dc0b

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
fmtstr_with_global_inliner_max -1 ✅ -0.95%
fmtstr_with_global_inliner_min -1 ✅ -0.95%
fmtstr_with_global_inliner_zero -1 ✅ -0.95%

Full diff report 👇
Program Brillig opcodes (+/-) %
debug_logs_inliner_min 5,341 (-1) -0.02%
debug_logs_inliner_zero 5,341 (-1) -0.02%
debug_logs_inliner_max 5,128 (-1) -0.02%
fmtstr_with_global_inliner_max 104 (-1) -0.95%
fmtstr_with_global_inliner_min 104 (-1) -0.95%
fmtstr_with_global_inliner_zero 104 (-1) -0.95%

@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2025

Changes to Brillig bytecode sizes

Generated at commit: b8c323e1b3c41947ccf9d2aed9a045b6a7a63671, compared to commit: 7902a216bd9a052ec57aaad6071bbaa779d4dc0b

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
fmtstr_with_global_inliner_max -1 ✅ -0.94%
fmtstr_with_global_inliner_min -1 ✅ -0.94%
fmtstr_with_global_inliner_zero -1 ✅ -0.94%

Full diff report 👇
Program Brillig opcodes (+/-) %
lambda_from_dynamic_if_inliner_zero 438 (+1) +0.23%
lambda_from_dynamic_if_inliner_min 446 (+1) +0.22%
lambda_from_dynamic_if_inliner_max 492 (+1) +0.20%
debug_logs_inliner_min 5,317 (-1) -0.02%
debug_logs_inliner_zero 5,317 (-1) -0.02%
debug_logs_inliner_max 5,116 (-1) -0.02%
fmtstr_with_global_inliner_max 105 (-1) -0.94%
fmtstr_with_global_inliner_min 105 (-1) -0.94%
fmtstr_with_global_inliner_zero 105 (-1) -0.94%

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: 5da5211 Previous: 7902a21 Ratio
rollup-merge 0.004 s 0.003 s 1.33

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.

LGTM

@asterite asterite enabled auto-merge June 13, 2025 13:41
@asterite asterite added this pull request to the merge queue Jun 13, 2025
Merged via the queue into master with commit abef727 Jun 13, 2025
118 checks passed
@asterite asterite deleted the ab/ssa-interpreter-fixes-2 branch June 13, 2025 14:18
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 18, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(LSP): suggest generic type methods
(noir-lang/noir#8948)
fix: Fix if/match tracking in last uses pass
(noir-lang/noir#8935)
fix(ssa): Swap Brillig index shift and DIE in minimal pipeline
(noir-lang/noir#8946)
fix: when macro parse error happens, discard warnings; also preserve
unquoted token locations (noir-lang/noir#8944)
chore: disable noncritical workflow steps which fail on external prs
(noir-lang/noir#8939)
chore(test): Add `--minimal-ssa` to integration tests
(noir-lang/noir#8938)
fix(noirc_evaluator): U128 Binary::And simplification
(noir-lang/noir#8940)
chore: Improve access to data required to extract Noir
(noir-lang/noir#8793)
chore: bump external pinned commits
(noir-lang/noir#8920)
chore(fuzz): Compare print output on failed programs
(noir-lang/noir#8921)
chore(fuzz): Display nightly fuzz status badge
(noir-lang/noir#8932)
fix(fuzz): Consider values returned from Brillig to ACIR as dynamic
(noir-lang/noir#8931)
fix(fuzz): Fix env var name in fuzzing workflow
(noir-lang/noir#8929)
chore(fuzz): Add nightly fuzz workflow with 5 minute budget
(noir-lang/noir#8925)
fix(mem2reg): Keep last store for a used nested array
(noir-lang/noir#8917)
chore(fuzz): Increase the number of deterministic test cases
(noir-lang/noir#8872)
fix: Create calls to `apply` before function values are changed to
fields in defunctionalize (noir-lang/noir#8916)
chore: Add various regression tests for mutable references
(noir-lang/noir#8915)
fix: handle return_data in the interpreter SSA CLI
(noir-lang/noir#8914)
feat(fuzz): Generate references in the AST fuzzer
(noir-lang/noir#8728)
fix(fuzz): Use an inline block to circumvent negation with overflow
(noir-lang/noir#8911)
fix: Inline global arrays with functions at their call site
(noir-lang/noir#8905)
chore: Casting ACIR vs Brillig regression tests
(noir-lang/noir#8903)
fix(mem2reg): Keep last store for reference in array used only in an
array get (noir-lang/noir#8877)
fix: more SSA interpreter fixes
(noir-lang/noir#8904)
chore: Ban dynamic array indices with reference elements
(noir-lang/noir#8888)
chore(ssa_fuzzer): Add SSA validation to the SSA fuzzer
(noir-lang/noir#8901)
chore(fuzz): More metamorphic rules
(noir-lang/noir#8857)
fix: catch unbound type variables during frontend compilation
(noir-lang/noir#8686)
fix: preserve functions which are used in `array_set` instructions
(noir-lang/noir#8891)
fix: assorted SSA interpreter fixes
(noir-lang/noir#8893)
fix(ssa): Signed cast simplification
(noir-lang/noir#8862)
feat: simplify apply function cfg immediately
(noir-lang/noir#8895)
fix: do not hoist control dependent cast
(noir-lang/noir#8886)
fix(licm): Account for negative bounds when checking whether a loop
executes (noir-lang/noir#8889)
chore: Release Noir(1.0.0-beta.7)
(noir-lang/noir#8520)
chore: test noir-ecdsa library in CI
(noir-lang/noir#8859)
chore: Incorrect clippy warning blocking PRs
(noir-lang/noir#8861)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(LSP): suggest generic type methods
(noir-lang/noir#8948)
fix: Fix if/match tracking in last uses pass
(noir-lang/noir#8935)
fix(ssa): Swap Brillig index shift and DIE in minimal pipeline
(noir-lang/noir#8946)
fix: when macro parse error happens, discard warnings; also preserve
unquoted token locations (noir-lang/noir#8944)
chore: disable noncritical workflow steps which fail on external prs
(noir-lang/noir#8939)
chore(test): Add `--minimal-ssa` to integration tests
(noir-lang/noir#8938)
fix(noirc_evaluator): U128 Binary::And simplification
(noir-lang/noir#8940)
chore: Improve access to data required to extract Noir
(noir-lang/noir#8793)
chore: bump external pinned commits
(noir-lang/noir#8920)
chore(fuzz): Compare print output on failed programs
(noir-lang/noir#8921)
chore(fuzz): Display nightly fuzz status badge
(noir-lang/noir#8932)
fix(fuzz): Consider values returned from Brillig to ACIR as dynamic
(noir-lang/noir#8931)
fix(fuzz): Fix env var name in fuzzing workflow
(noir-lang/noir#8929)
chore(fuzz): Add nightly fuzz workflow with 5 minute budget
(noir-lang/noir#8925)
fix(mem2reg): Keep last store for a used nested array
(noir-lang/noir#8917)
chore(fuzz): Increase the number of deterministic test cases
(noir-lang/noir#8872)
fix: Create calls to `apply` before function values are changed to
fields in defunctionalize (noir-lang/noir#8916)
chore: Add various regression tests for mutable references
(noir-lang/noir#8915)
fix: handle return_data in the interpreter SSA CLI
(noir-lang/noir#8914)
feat(fuzz): Generate references in the AST fuzzer
(noir-lang/noir#8728)
fix(fuzz): Use an inline block to circumvent negation with overflow
(noir-lang/noir#8911)
fix: Inline global arrays with functions at their call site
(noir-lang/noir#8905)
chore: Casting ACIR vs Brillig regression tests
(noir-lang/noir#8903)
fix(mem2reg): Keep last store for reference in array used only in an
array get (noir-lang/noir#8877)
fix: more SSA interpreter fixes
(noir-lang/noir#8904)
chore: Ban dynamic array indices with reference elements
(noir-lang/noir#8888)
chore(ssa_fuzzer): Add SSA validation to the SSA fuzzer
(noir-lang/noir#8901)
chore(fuzz): More metamorphic rules
(noir-lang/noir#8857)
fix: catch unbound type variables during frontend compilation
(noir-lang/noir#8686)
fix: preserve functions which are used in `array_set` instructions
(noir-lang/noir#8891)
fix: assorted SSA interpreter fixes
(noir-lang/noir#8893)
fix(ssa): Signed cast simplification
(noir-lang/noir#8862)
feat: simplify apply function cfg immediately
(noir-lang/noir#8895)
fix: do not hoist control dependent cast
(noir-lang/noir#8886)
fix(licm): Account for negative bounds when checking whether a loop
executes (noir-lang/noir#8889)
chore: Release Noir(1.0.0-beta.7)
(noir-lang/noir#8520)
chore: test noir-ecdsa library in CI
(noir-lang/noir#8859)
chore: Incorrect clippy warning blocking PRs
(noir-lang/noir#8861)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
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.

SSA interpreter CLI can't handle composite input types

2 participants