Skip to content

chore(SSA): restrict shr and shl right-hand side to u8#8753

Merged
asterite merged 3 commits intomasterfrom
ab/ssa-shr-enforce-u8
Jun 3, 2025
Merged

chore(SSA): restrict shr and shl right-hand side to u8#8753
asterite merged 3 commits intomasterfrom
ab/ssa-shr-enforce-u8

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented Jun 2, 2025

Description

Problem

Resolves #8544

Summary

Also added a check that for other binary operations the types of lhs and rhs is the same.

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.

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: dbf5726 Previous: 5984575 Ratio
test_report_zkpassport_noir_rsa_ 3 s 2 s 1.50

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

CC: @TomAFrench

@asterite asterite marked this pull request as draft June 2, 2025 13:05
@asterite asterite changed the title chore(SSA): restrict shr right-hand side to u8 chore(SSA): restrict shr and shrl right-hand side to u8 Jun 2, 2025
@asterite asterite changed the title chore(SSA): restrict shr and shrl right-hand side to u8 chore(SSA): restrict shr and shl right-hand side to u8 Jun 2, 2025
@asterite asterite marked this pull request as ready for review June 2, 2025 14:37
@asterite asterite added the bench-show Display benchmark results on PR label Jun 2, 2025
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.

ACVM Benchmarks

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
purely_sequential_opcodes 254469 ns/iter (± 612) 254157 ns/iter (± 767) 1.00
perfectly_parallel_opcodes 224089 ns/iter (± 1787) 223920 ns/iter (± 3273) 1.00
perfectly_parallel_batch_inversion_opcodes 3210449 ns/iter (± 2994) 3211270 ns/iter (± 19443) 1.00

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

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.

Execution Time

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
private-kernel-inner 0.028 s 0.028 s 1
private-kernel-reset 0.162 s 0.163 s 0.99
private-kernel-tail 0.011 s 0.011 s 1
rollup-base-private 0.301 s 0.304 s 0.99
rollup-base-public 0.192 s 0.195 s 0.98
rollup-block-root 11 s 11.1 s 0.99
rollup-merge 0.004 s 0.004 s 1
rollup-root 0.009 s 0.009 s 1
semaphore-depth-10 0.02 s 0.02 s 1

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

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.

Compilation Time

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
private-kernel-inner 2.31 s 2.328 s 0.99
private-kernel-reset 7.68 s 7.702 s 1.00
private-kernel-tail 1.074 s 1.09 s 0.99
rollup-base-private 16.28 s 16.6 s 0.98
rollup-base-public 13.42 s 13.42 s 1
rollup-block-root-empty 1.23 s 1.262 s 0.97
rollup-block-root-single-tx 122 s 120 s 1.02
rollup-block-root 124 s 120 s 1.03
rollup-merge 1.08 s 1.082 s 1.00
rollup-root 1.576 s 1.536 s 1.03
semaphore-depth-10 0.809 s 0.824 s 0.98

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

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.

Artifact Size

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
private-kernel-inner 1133 KB 1133 KB 1
private-kernel-reset 2064.4 KB 2064.4 KB 1
private-kernel-tail 588.8 KB 588.8 KB 1
rollup-base-private 4955.9 KB 4955.9 KB 1
rollup-base-public 3995.3 KB 3995.3 KB 1
rollup-block-root-empty 256.7 KB 256.7 KB 1
rollup-block-root-single-tx 25707.5 KB 25707.5 KB 1
rollup-block-root 25714.4 KB 25714.4 KB 1
rollup-merge 183.4 KB 183.4 KB 1
rollup-root 417.9 KB 417.9 KB 1
semaphore-depth-10 636.3 KB 636.3 KB 1

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

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.

Test Suite Duration

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr 70 s 67 s 1.04
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts 106 s 107 s 0.99
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 44 s 45 s 0.98
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 203 s 221 s 0.92
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib 246 s 245 s 1.00
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types 70 s 69 s 1.01
test_report_noir-lang_noir-bignum_ 379 s 370 s 1.02
test_report_noir-lang_noir_bigcurve_ 248 s 246 s 1.01
test_report_noir-lang_sha512_ 31 s 31 s 1
test_report_zkpassport_noir_rsa_ 2 s 2 s 1

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

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.

Compilation Memory

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
private-kernel-inner 292.9 MB 292.99 MB 1.00
private-kernel-reset 533.91 MB 533.91 MB 1
private-kernel-tail 192.8 MB 192.81 MB 1.00
rollup-base-private 1380 MB 1380 MB 1
rollup-base-public 1540 MB 1540 MB 1
rollup-block-root-empty 342.94 MB 342.95 MB 1.00
rollup-block-root-single-tx 7150 MB 7150 MB 1
rollup-block-root 7160 MB 7160 MB 1
rollup-merge 327.16 MB 327.16 MB 1
rollup-root 381.5 MB 381.45 MB 1.00
semaphore_depth_10 106.4 MB 106.4 MB 1

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

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.

Execution Memory

Details
Benchmark suite Current: d034883 Previous: 5984575 Ratio
private-kernel-inner 202.02 MB 202.02 MB 1
private-kernel-reset 225.76 MB 225.76 MB 1
private-kernel-tail 177.16 MB 177.16 MB 1
rollup-base-private 489.95 MB 489.94 MB 1.00
rollup-base-public 423.45 MB 423.45 MB 1
rollup-block-root 1400 MB 1400 MB 1
rollup-merge 312.01 MB 312.01 MB 1
rollup-root 317.7 MB 317.7 MB 1
semaphore_depth_10 70.97 MB 70.97 MB 1

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

@asterite asterite requested a review from a team June 2, 2025 15:24
Copy link
Contributor

@vezenovm vezenovm left a comment

Choose a reason for hiding this comment

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

I'm good with merging this as is, but I realize now this is something I missed when creating the separate validator module #8765. We could merge this and I update #8765 afterwards or vice-versa.

@asterite
Copy link
Collaborator Author

asterite commented Jun 3, 2025

Let's merge this first. It's a small change so it should be easy to merge into the other PR.

@asterite asterite added this pull request to the merge queue Jun 3, 2025
Merged via the queue into master with commit 03796c0 Jun 3, 2025
117 checks passed
@asterite asterite deleted the ab/ssa-shr-enforce-u8 branch June 3, 2025 12:52
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 6, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: type unification tests, and try moving constants to the other side
(noir-lang/noir#8807)
fix!: disallow casting numeric to bool
(noir-lang/noir#8703)
fix: delay associated constants resolution
(noir-lang/noir#8744)
fix: right shift overflow to 0
(noir-lang/noir#8772)
fix: use value predicated by range checks
(noir-lang/noir#8778)
fix: unify infix expressions by isolating unbound type variables
(noir-lang/noir#8796)
feat: use `asm` feature flag in arkworks
(noir-lang/noir#8792)
chore: add a failing test for #8780
(noir-lang/noir#8785)
chore: Adjust the frequency of 'for' statements in ACIR fuzz generation
(noir-lang/noir#8788)
fix: Merge `replacement_type` and `is_function_type` in
`defunctionalization` (noir-lang/noir#8784)
chore(fuzz): Remove unreachable functions in the AST fuzzer
(noir-lang/noir#8782)
chore(docs): Copy attribute docs into versioned docs
(noir-lang/noir#8777)
fix(licm): Preserve semantic ordering of side-effectual instructions
when hoisting (noir-lang/noir#8724)
fix: Create SSA interpreter arguments from scratch for each invocation
(noir-lang/noir#8762)
chore: mark sha512 as non-critical
(noir-lang/noir#8776)
fix!: disallow specifying associated items via generics
(noir-lang/noir#8756)
fix: stop inserting instructions after break and continue
(noir-lang/noir#8712)
fix: Fix comptime casts of negative integer to field
(noir-lang/noir#8696)
chore(SSA): restrict `shr` and `shl` right-hand side to u8
(noir-lang/noir#8753)
chore: bump some JS packages
(noir-lang/noir#8771)
chore: document `allow(dead_code)` and reorganize attributes
(noir-lang/noir#8766)
fix: Add missing cases for finding function values in
`find_functions_as_values` (noir-lang/noir#8738)
fix: correct bitsize in signed division
(noir-lang/noir#8733)
chore: remove noir-lang/noir_rsa from external libraries
(noir-lang/noir#8752)
chore: bump external pinned commits
(noir-lang/noir#8747)
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: type unification tests, and try moving constants to the other side
(noir-lang/noir#8807)
fix!: disallow casting numeric to bool
(noir-lang/noir#8703)
fix: delay associated constants resolution
(noir-lang/noir#8744)
fix: right shift overflow to 0
(noir-lang/noir#8772)
fix: use value predicated by range checks
(noir-lang/noir#8778)
fix: unify infix expressions by isolating unbound type variables
(noir-lang/noir#8796)
feat: use `asm` feature flag in arkworks
(noir-lang/noir#8792)
chore: add a failing test for AztecProtocol#8780
(noir-lang/noir#8785)
chore: Adjust the frequency of 'for' statements in ACIR fuzz generation
(noir-lang/noir#8788)
fix: Merge `replacement_type` and `is_function_type` in
`defunctionalization` (noir-lang/noir#8784)
chore(fuzz): Remove unreachable functions in the AST fuzzer
(noir-lang/noir#8782)
chore(docs): Copy attribute docs into versioned docs
(noir-lang/noir#8777)
fix(licm): Preserve semantic ordering of side-effectual instructions
when hoisting (noir-lang/noir#8724)
fix: Create SSA interpreter arguments from scratch for each invocation
(noir-lang/noir#8762)
chore: mark sha512 as non-critical
(noir-lang/noir#8776)
fix!: disallow specifying associated items via generics
(noir-lang/noir#8756)
fix: stop inserting instructions after break and continue
(noir-lang/noir#8712)
fix: Fix comptime casts of negative integer to field
(noir-lang/noir#8696)
chore(SSA): restrict `shr` and `shl` right-hand side to u8
(noir-lang/noir#8753)
chore: bump some JS packages
(noir-lang/noir#8771)
chore: document `allow(dead_code)` and reorganize attributes
(noir-lang/noir#8766)
fix: Add missing cases for finding function values in
`find_functions_as_values` (noir-lang/noir#8738)
fix: correct bitsize in signed division
(noir-lang/noir#8733)
chore: remove noir-lang/noir_rsa from external libraries
(noir-lang/noir#8752)
chore: bump external pinned commits
(noir-lang/noir#8747)
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

bench-show Display benchmark results on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACIR vs Brillig: bool shr

2 participants