Skip to content
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: more arithmetic optimizations #2647

Merged
merged 47 commits into from
May 10, 2022

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Feb 12, 2022

What I did

How I did it

How to verify it

tests pass

Commit Message

    refactor IR optimizer
    
    this is a small rewrite of the IR optimizer. it changes the structure of
    the binop optimizations so that it is easier to add more optimizations.
    it also refactors the `clamp` optimizations to be in terms of an
    `assert` statement, so that the clamp conditions can be optimized using
    the binop optimizer code.

Description for the changelog

Cute Animal Picture

image

@charles-cooper charles-cooper force-pushed the rewrite_optimizer3 branch 2 times, most recently from cdca867 to 4e71f13 Compare February 15, 2022 14:01
@charles-cooper charles-cooper changed the title Rewrite optimizer3 add some arithmetic optimizations Feb 15, 2022
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 15, 2022

This pull request introduces 1 alert when merging 38bdfad into 43f491f - view on LGTM.com

new alerts:

  • 1 for Syntax error

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 1, 2022

This pull request introduces 11 alerts when merging abfadc7 into 4d32d17 - view on LGTM.com

new alerts:

  • 8 for Unused local variable
  • 2 for Wrong number of arguments in a call
  • 1 for Redundant assignment

@codecov-commenter
Copy link

codecov-commenter commented May 1, 2022

Codecov Report

Merging #2647 (2a5cc04) into master (4b44ee7) will increase coverage by 0.07%.
The diff coverage is 91.91%.

❗ Current head 2a5cc04 differs from pull request most recent head 689edc6. Consider uploading reports for the commit 689edc6 to get more accurate results

@@            Coverage Diff             @@
##           master    #2647      +/-   ##
==========================================
+ Coverage   87.50%   87.58%   +0.07%     
==========================================
  Files          94       94              
  Lines       10118    10158      +40     
  Branches     2491     2506      +15     
==========================================
+ Hits         8854     8897      +43     
+ Misses        801      792       -9     
- Partials      463      469       +6     
Impacted Files Coverage Δ
vyper/codegen/stmt.py 87.90% <ø> (-0.20%) ⬇️
vyper/ir/compile_ir.py 93.62% <ø> (-0.98%) ⬇️
vyper/utils.py 87.15% <85.71%> (+0.32%) ⬆️
vyper/ir/optimizer.py 91.50% <90.71%> (+11.50%) ⬆️
vyper/builtin_functions/convert.py 90.45% <100.00%> (-0.04%) ⬇️
vyper/builtin_functions/functions.py 89.23% <100.00%> (ø)
vyper/codegen/core.py 84.85% <100.00%> (+0.29%) ⬆️
vyper/codegen/expr.py 80.73% <100.00%> (ø)
vyper/codegen/ir_node.py 92.80% <100.00%> (-0.03%) ⬇️
vyper/codegen/return_.py 94.87% <100.00%> (ø)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b44ee7...689edc6. Read the comment docs.

this is a small rewrite of the IR optimizer. it changes the structure of
the binop optimizations so that it is easier to add more optimizations.
it also refactors the `clamp` optimizations to be in terms of an
`assert` statement, so that the clamp conditions can be optimized using
the binop optimizer code.
@charles-cooper charles-cooper marked this pull request as ready for review May 3, 2022 14:12
replaced with a utility codegen routine
it can backfire if the condition already has an iszero in it, e.g.
```
if (iszero x) {if_branch} {else_branch}
```
would get rewritten to
```
if (iszero (iszero x)) {else_branch} {if_branch}
```
which still has an iszero in the final assembly!
optimizations for `sub` are probably better.
Copy link
Contributor

@skellet0r skellet0r left a comment

Choose a reason for hiding this comment

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

🚀 amazing

vyper/ir/optimizer.py Show resolved Hide resolved
vyper/ir/optimizer.py Outdated Show resolved Hide resolved
Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
@charles-cooper charles-cooper enabled auto-merge (squash) May 10, 2022 08:52
@charles-cooper charles-cooper changed the title add some arithmetic optimizations feat: more arithmetic optimizations May 10, 2022
@charles-cooper charles-cooper merged commit efe1dbe into vyperlang:master May 10, 2022
tserg added a commit to tserg/vyper that referenced this pull request May 13, 2022
this is a small rewrite of the IR optimizer. it changes the structure of
the binop optimizations so that it is easier to add more optimizations.
it also refactors the `clamp` optimizations to be in terms of an
`assert` statement, so that the clamp conditions can be optimized using
the binop optimizer code.

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
@charles-cooper charles-cooper deleted the rewrite_optimizer3 branch May 25, 2022 09:29
charles-cooper added a commit that referenced this pull request May 25, 2022
@emc415 emc415 mentioned this pull request Dec 19, 2022
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.

None yet

4 participants