Skip to content

feat: simplify bitshift logic to remove leftover overflow handling#9506

Merged
TomAFrench merged 11 commits intomasterfrom
tf/bitshift-simplification
Aug 16, 2025
Merged

feat: simplify bitshift logic to remove leftover overflow handling#9506
TomAFrench merged 11 commits intomasterfrom
tf/bitshift-simplification

Conversation

@TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

After #9373, we still have some leftover logic where we're handling the potential for bitshifts greater than the type size. This PR removes some of those cases as well as doing a few simplifications to the pow function to reduce unnecessary DOF

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 15, 2025

Changes to circuit sizes

Generated at commit: bcb40a8ac77a724350de238b18d8cb4e5186d01a, compared to commit: 12465f3ae45cdbd96531889e9420ad02254e17f9

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
bit_shifts_runtime -82 ✅ -13.10% -139 ✅ -2.76%
binary_operator_overloading -7 ✅ -7.07% -711 ✅ -17.70%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
inactive_signed_bitshift 80 (-16) -16.67% 2,849 (-15) -0.52%
u16_support 38 (-7) -15.56% 2,797 (-29) -1.03%
bit_shifts_runtime 544 (-82) -13.10% 4,895 (-139) -2.76%
binary_operator_overloading 92 (-7) -7.07% 3,305 (-711) -17.70%

@TomAFrench TomAFrench requested a review from a team August 15, 2025 13:51
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: 16ad037 Previous: 12465f3 Ratio
test_report_zkpassport_noir_rsa_ 1 s 0 s +∞

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 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: f121f90 Previous: bae9236 Ratio
sha512-100-bytes 2.013 s 1.539 s 1.31

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: 68e6bad Previous: 29160ca Ratio
private-kernel-inner 0.017 s 0.014 s 1.21

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

CC: @TomAFrench

@TomAFrench TomAFrench force-pushed the tf/bitshift-simplification branch from d34f12c to 16ad037 Compare August 15, 2025 21:14
@TomAFrench TomAFrench enabled auto-merge August 15, 2025 21:14
@TomAFrench TomAFrench added this pull request to the merge queue Aug 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 15, 2025
@TomAFrench TomAFrench added this pull request to the merge queue Aug 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 16, 2025
@TomAFrench TomAFrench added this pull request to the merge queue Aug 16, 2025
Merged via the queue into master with commit 9062697 Aug 16, 2025
122 checks passed
@TomAFrench TomAFrench deleted the tf/bitshift-simplification branch August 16, 2025 13:39
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.

2 participants