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[venom]: new DFTPass algorithm #4255

Draft
wants to merge 83 commits into
base: master
Choose a base branch
from

Conversation

harkal
Copy link
Collaborator

@harkal harkal commented Sep 24, 2024

What I did

I upgraded the DFT algorithm to allow for more instruction movement. It also removes the use of order ids and sorting. It results generally smaller code, and opens up the road for multi-dimensional fencing.

How I did it

How to verify it

Commit message

This commit implements a new `DFTPass` to handle instruction grouping and 
ordering based on dependencies. This new algorithm ensures that instructions 
are processed in a dependency-first traversal manner as groups but also
individually, with special handling for volatile instructions and inter-group 
dependencies.

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

vyper/venom/passes/dft.py Fixed Show fixed Hide fixed
#
# Apply effects to dependency graph
#
last_effects = {}

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable last_effects is not used.
#
last_effects = {}
last_volatile = None
terminator_inst = non_phis[-1]

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable terminator_inst is not used.
Comment on lines 113 to 120
# for write_effect in write_effects:
# if write_effect in last_effects:
# self.ida[inst].append(last_effects[write_effect])
# last_effects[write_effect] = inst

self.fence_id = 0
# for read_effect in read_effects:
# if read_effect in last_effects:
# self.ida[inst].append(last_effects[read_effect])

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
@@ -239,6 +236,34 @@
@property
def is_bb_terminator(self) -> bool:
return self.opcode in BB_TERMINATORS

def get_read_effects(self):

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'get_read_effects' is unnecessary as it is
redefined
before this value is used.
def get_read_effects(self):
return effects.reads.get(self.opcode, ())

def get_write_effects(self):

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'get_write_effects' is unnecessary as it is
redefined
before this value is used.
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