Skip to content

feat!: new semantic for bit-shifts #9373

Merged
TomAFrench merged 45 commits intomasterfrom
gd/issue_9022
Aug 14, 2025
Merged

feat!: new semantic for bit-shifts #9373
TomAFrench merged 45 commits intomasterfrom
gd/issue_9022

Conversation

@guipublic
Copy link
Contributor

Description

Problem*

Resolves #9022

Summary*

Bit-shifts now requires that rhs and lhs have the same type (instead of u8 for rhs). This is a breaking change.
If rhs > bit_size of rhs type (or bit_size -1 for signed types), then bit-shift will overflow.

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 Aug 1, 2025

Changes to Brillig bytecode sizes

Generated at commit: 8172f2128350a775bc9e738b4c8173ceaa6f4cc7, compared to commit: 4f8dbbc20bb40115d87c499154655ebe7e962099

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
bit_shifts_runtime_inliner_max +59 ❌ +10.99%
bit_shifts_runtime_inliner_min +59 ❌ +10.99%
bit_shifts_runtime_inliner_zero +59 ❌ +10.99%
shift_right_overflow_inliner_max -7 ✅ -24.14%
shift_right_overflow_inliner_min -7 ✅ -24.14%
shift_right_overflow_inliner_zero -7 ✅ -24.14%

Full diff report 👇
Program Brillig opcodes (+/-) %
bit_shifts_runtime_inliner_max 596 (+59) +10.99%
bit_shifts_runtime_inliner_min 596 (+59) +10.99%
bit_shifts_runtime_inliner_zero 596 (+59) +10.99%
conditional_regression_underflow_inliner_max 68 (+6) +9.68%
conditional_regression_underflow_inliner_min 68 (+6) +9.68%
conditional_regression_underflow_inliner_zero 68 (+6) +9.68%
bit_shifts_comptime_inliner_max 119 (+2) +1.71%
bit_shifts_comptime_inliner_min 119 (+2) +1.71%
bit_shifts_comptime_inliner_zero 119 (+2) +1.71%
array_dynamic_blackbox_input_inliner_min 388 (-1) -0.26%
array_dynamic_blackbox_input_inliner_max 367 (-1) -0.27%
array_dynamic_blackbox_input_inliner_zero 367 (-1) -0.27%
u128_type_inliner_max 138 (-1) -0.72%
u128_type_inliner_min 138 (-1) -0.72%
u128_type_inliner_zero 138 (-1) -0.72%
binary_operator_overloading_inliner_max 380 (-3) -0.78%
binary_operator_overloading_inliner_min 221 (-3) -1.34%
binary_operator_overloading_inliner_zero 221 (-3) -1.34%
u16_support_inliner_min 56 (-3) -5.08%
u16_support_inliner_zero 56 (-3) -5.08%
u16_support_inliner_max 48 (-3) -5.88%
shift_right_overflow_inliner_max 22 (-7) -24.14%
shift_right_overflow_inliner_min 22 (-7) -24.14%
shift_right_overflow_inliner_zero 22 (-7) -24.14%

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

Changes to number of Brillig opcodes executed

Generated at commit: 8172f2128350a775bc9e738b4c8173ceaa6f4cc7, compared to commit: 4f8dbbc20bb40115d87c499154655ebe7e962099

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
bit_shifts_runtime_inliner_max +677 ❌ +298.24%
bit_shifts_runtime_inliner_min +677 ❌ +298.24%
bit_shifts_runtime_inliner_zero +677 ❌ +298.24%
shift_right_overflow_inliner_max -5 ✅ -20.00%
shift_right_overflow_inliner_min -5 ✅ -20.00%
shift_right_overflow_inliner_zero -5 ✅ -20.00%

Full diff report 👇
Program Brillig opcodes (+/-) %
bit_shifts_runtime_inliner_max 904 (+677) +298.24%
bit_shifts_runtime_inliner_min 904 (+677) +298.24%
bit_shifts_runtime_inliner_zero 904 (+677) +298.24%
conditional_regression_underflow_inliner_max 36 (+4) +12.50%
conditional_regression_underflow_inliner_min 36 (+4) +12.50%
conditional_regression_underflow_inliner_zero 36 (+4) +12.50%
array_dynamic_blackbox_input_inliner_min 6,557 (-1) -0.02%
array_dynamic_blackbox_input_inliner_max 6,528 (-1) -0.02%
array_dynamic_blackbox_input_inliner_zero 6,528 (-1) -0.02%
bit_shifts_comptime_inliner_max 462 (-2) -0.43%
bit_shifts_comptime_inliner_min 462 (-2) -0.43%
bit_shifts_comptime_inliner_zero 462 (-2) -0.43%
u128_type_inliner_max 92 (-1) -1.08%
u128_type_inliner_min 92 (-1) -1.08%
u128_type_inliner_zero 92 (-1) -1.08%
binary_operator_overloading_inliner_min 225 (-3) -1.32%
binary_operator_overloading_inliner_zero 225 (-3) -1.32%
binary_operator_overloading_inliner_max 209 (-3) -1.42%
u16_support_inliner_min 48 (-3) -5.88%
u16_support_inliner_zero 48 (-3) -5.88%
u16_support_inliner_max 36 (-3) -7.69%
shift_right_overflow_inliner_max 20 (-5) -20.00%
shift_right_overflow_inliner_min 20 (-5) -20.00%
shift_right_overflow_inliner_zero 20 (-5) -20.00%

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

FYI @noir-lang/developerrelations on Noir doc changes.

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: 925d517 Previous: 4f8dbbc Ratio
test_report_zkpassport_noir-ecdsa_ 2 s 1 s 2

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

CC: @TomAFrench

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 1, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 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.

⚠️ 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: d2200ea Previous: ee2ac1a Ratio
sha512-100-bytes 0.142 s 0.096 s 1.48

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

CC: @TomAFrench

@guipublic guipublic mentioned this pull request Aug 4, 2025
5 tasks
@jfecher
Copy link
Contributor

jfecher commented Aug 4, 2025

Looks like this is timing out all the debug tests locally

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.

I think we've got some undesirable behaviour here which means that 4u8>>2u8 is fine but 1u8>>2u8 is disallowed. I would expect the latter to result in zero. I was misreading comments.

@TomAFrench
Copy link
Member

TomAFrench commented Aug 8, 2025

Program Brillig opcodes (+/-) %
bit_shifts_runtime_inliner_max +677 ❌ +298.24%
bit_shifts_runtime_inliner_min +677 ❌ +298.24%
bit_shifts_runtime_inliner_zero +677 ❌ +298.24%

Strange we're getting such a large regression here as while there's been a few changes to this program it shouldn't result in ~700 new opcodes afaict.

Edit: due to the removals above?

Edit Edit: We're now doing more runtime signed bitshifts so the overflow checks could be the cause of this.

@TomAFrench TomAFrench changed the title chore!: new semantic for bit-shifts feat!: new semantic for bit-shifts Aug 13, 2025
@TomAFrench TomAFrench requested a review from a team August 13, 2025 14:37
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 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d9ba69c Previous: ee2ac1a Ratio
semaphore-depth-10 0.958 s 0.759 s 1.26

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 - looks like external repos are failing to build though

@TomAFrench TomAFrench enabled auto-merge August 14, 2025 10:22
@TomAFrench TomAFrench added this pull request to the merge queue Aug 14, 2025
Merged via the queue into master with commit f6fed8b Aug 14, 2025
101 checks passed
@TomAFrench TomAFrench deleted the gd/issue_9022 branch August 14, 2025 16:52
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 15, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: restore noir libs in CI (noir-lang/noir#9499)
feat!: new semantic for bit-shifts  (noir-lang/noir#9373)
fix(ssa): Replace side effects with defaults when disabled (noir-lang/noir#9462)
fix(ssa): Replace pop from 0-length slice with constraint and defaults (noir-lang/noir#9489)
fix: assert types are not mutated in constant folding (noir-lang/noir#9481)
fix: remove shadowing in `BoundedVec::any` causing returning  false unconditionally (noir-lang/noir#9478)
END_COMMIT_OVERRIDE
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 15, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: restore noir libs in CI (noir-lang/noir#9499)
feat!: new semantic for bit-shifts  (noir-lang/noir#9373)
fix(ssa): Replace side effects with defaults when disabled (noir-lang/noir#9462)
fix(ssa): Replace pop from 0-length slice with constraint and defaults (noir-lang/noir#9489)
fix: assert types are not mutated in constant folding (noir-lang/noir#9481)
fix: remove shadowing in `BoundedVec::any` causing returning  false unconditionally (noir-lang/noir#9478)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 15, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: restore noir libs in CI
(noir-lang/noir#9499)
feat!: new semantic for bit-shifts
(noir-lang/noir#9373)
fix(ssa): Replace side effects with defaults when disabled
(noir-lang/noir#9462)
fix(ssa): Replace pop from 0-length slice with constraint and defaults
(noir-lang/noir#9489)
fix: assert types are not mutated in constant folding
(noir-lang/noir#9481)
fix: remove shadowing in `BoundedVec::any` causing returning false
unconditionally (noir-lang/noir#9478)
END_COMMIT_OVERRIDE
mralj 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: restore noir libs in CI (noir-lang/noir#9499)
feat!: new semantic for bit-shifts  (noir-lang/noir#9373)
fix(ssa): Replace side effects with defaults when disabled (noir-lang/noir#9462)
fix(ssa): Replace pop from 0-length slice with constraint and defaults (noir-lang/noir#9489)
fix: assert types are not mutated in constant folding (noir-lang/noir#9481)
fix: remove shadowing in `BoundedVec::any` causing returning  false unconditionally (noir-lang/noir#9478)
END_COMMIT_OVERRIDE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shift-left inconsistency

4 participants