-
-
Notifications
You must be signed in to change notification settings - Fork 795
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
refactor[venom]: move dfs order to CFGAnalysis
#4330
base: master
Are you sure you want to change the base?
refactor[venom]: move dfs order to CFGAnalysis
#4330
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4330 +/- ##
===========================================
- Coverage 91.30% 46.13% -45.18%
===========================================
Files 112 112
Lines 15921 15916 -5
Branches 2691 2684 -7
===========================================
- Hits 14537 7343 -7194
- Misses 949 8018 +7069
- Partials 435 555 +120 ☔ View full report in Codecov by Sentry. |
it is redundant with CFGAnalysis cfg_out
this shaves 1.5% off bytecode size
@@ -1,8 +1,7 @@ | |||
from typing import Iterator, Optional | |||
|
|||
from vyper.codegen.ir_node import IRnode | |||
from vyper.utils import OrderedSet | |||
from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS, IRBasicBlock, IRLabel, IRVariable | |||
from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
vyper.venom.basicblock
vyper.venom.function
definition
import
@@ -1,8 +1,7 @@ | |||
from typing import Iterator, Optional | |||
|
|||
from vyper.codegen.ir_node import IRnode | |||
from vyper.utils import OrderedSet | |||
from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS, IRBasicBlock, IRLabel, IRVariable | |||
from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
vyper.venom.basicblock
vyper.venom.function
definition
import
@@ -1,8 +1,7 @@ | |||
from typing import Iterator, Optional | |||
|
|||
from vyper.codegen.ir_node import IRnode | |||
from vyper.utils import OrderedSet | |||
from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS, IRBasicBlock, IRLabel, IRVariable | |||
from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
vyper.venom.basicblock
vyper.venom.function
definition
import
shared code between remove_unreachable_blocks and sccp
@@ -308,7 +314,7 @@ def _generate_evm_for_basicblock_r( | |||
|
|||
ref.extend(asm) | |||
|
|||
for bb in basicblock.reachable: | |||
for bb in basicblock.cfg_out: |
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.
Could be reachable removed from basic block if it is no longer used here?
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.
Yes, good point
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.
hmm, this creates a regression in |
What I did
move dfs order calculation from domtree to cfg analysis.
remove unnecessary calculation of domtree in sccp
remove redundant
IRFunction.compute_reachability
change cfg_out order
refactor shared phi fixup code
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture