-
-
Notifications
You must be signed in to change notification settings - Fork 880
feat[venom]: add assert combiner optimization pass #4818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat[venom]: add assert combiner optimization pass #4818
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4818 +/- ##
==========================================
- Coverage 93.35% 93.32% -0.03%
==========================================
Files 148 149 +1
Lines 20588 20698 +110
Branches 3574 3599 +25
==========================================
+ Hits 19220 19317 +97
- Misses 917 923 +6
- Partials 451 458 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
charles-cooper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please split this into an analysis pass (can be a local _AssertCombineAnalysis) and code modification pass
|
@codex do you work yet |
|
Codex couldn't complete this request. Try again later. |
Address review comment to separate analysis from code modification. Now has: - _MergeCandidate: dataclass for merge candidates - _AssertCombineAnalysis: identifies pairs of asserts that can be merged - AssertCombinerPass: applies the transformations
3f31223 to
2d5b6ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed review comment: split into analysis phase (_AssertCombineAnalysis) and transformation phase. All tests pass.
What I did
Add a new Venom optimization pass that combines adjacent
assert iszero(x)sequences into a single assert usingor, reducing bytecode size and gas cost.How I did it
The implementation is split into two phases per reviewer request:
_AssertCombineAnalysis): identifies pairs of asserts that can be merged based on compatible error messages and no side-effecting instructions between themAssertCombinerPass): applies the merges, combining predicates withorand removing redundant assertsKey features:
How to verify it
Run the unit tests:
Commit message
Description for the changelog
Add
AssertCombinerPassoptimization that combines adjacent assert conditions using bitwise OR.Cute Animal Picture