-
Notifications
You must be signed in to change notification settings - Fork 79
New dependency finding #683
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
Changes from 5 commits
3c01e0c
a5ee582
1a939a5
e553cce
ed6d03b
a10d3db
81b26a0
09899bf
464895f
d947d6b
fdb5a01
29ae7c0
0294761
6ed1cce
ccfca54
6e47dd0
a0fd274
2569006
d35f11b
a3a4c15
4d4ebdb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -80,6 +80,20 @@ class _BoundsRecord: | |||||
| upper_bound_pw_aff: isl.PwAff | ||||||
| size: isl.PwAff | ||||||
|
|
||||||
| @dataclass(frozen=True) | ||||||
| class RelationInfo: | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add into documentation. |
||||||
| insn_id: str | ||||||
| var_name: str | ||||||
| relation: isl.Map | ||||||
|
|
||||||
| @dataclass(frozen=True) | ||||||
| class AccessRelation(RelationInfo): | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should access relations be long-lived?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "has-a happier than is-a" |
||||||
| access_type: str | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this want to be an enum? |
||||||
|
|
||||||
| @dataclass(frozen=True) | ||||||
| class DependencyRelation(RelationInfo): | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something should be named "happens-before" |
||||||
| dependent_id: str | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make one end of the happens-before implicit by ownership---only one insn ID needs storing. |
||||||
| dependency_type: str | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wants to be an enum, possibly. |
||||||
|
|
||||||
| PreambleGenerator = Callable[["PreambleInfo"], Iterator[Tuple[int, str]]] | ||||||
|
|
||||||
|
|
@@ -640,6 +654,81 @@ def _remove_inames_for_shared_hw_axes(self, cond_inames): | |||||
|
|
||||||
| # {{{ dependency wrangling | ||||||
|
|
||||||
| def generate_access_relations(self): | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Things should be functions (not methods) unless absolutely necessary.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| from loopy.symbolic import BatchedAccessMapMapper | ||||||
| bmap = BatchedAccessMapMapper(self, self.all_variable_names()) | ||||||
| for insn in self.instructions: | ||||||
| bmap(insn.assignee, insn.within_inames) | ||||||
| bmap(insn.expression, insn.within_inames) | ||||||
|
|
||||||
| def get_map(var, insn): | ||||||
| return bmap.access_maps[var][insn.within_inames] | ||||||
|
|
||||||
| def read_var_list(insn): | ||||||
| return insn.read_dependency_names() - insn.within_inames | ||||||
|
|
||||||
| def write_var_list(insn): | ||||||
| return insn.write_dependency_names() - insn.within_inames | ||||||
|
|
||||||
| read_maps = [AccessRelation(insn.id, var, get_map(var, insn), "read") | ||||||
| for insn in self.instructions | ||||||
| for var in read_var_list(insn)] | ||||||
| write_maps = [AccessRelation(insn.id, var, get_map(var, insn), "write") | ||||||
| for insn in self.instructions | ||||||
| for var in write_var_list(insn)] | ||||||
|
|
||||||
| return read_maps, write_maps | ||||||
|
|
||||||
| def generate_dependency_relations(self, read_maps, write_maps): | ||||||
|
|
||||||
| def get_dependency_relation(x, y): | ||||||
|
|
||||||
| # TODO deal with self dependencies | ||||||
| #self_dependency = x.relation.apply_range(x.relation.reverse()) | ||||||
|
|
||||||
| dependency_relation = x.relation.apply_range(y.relation.reverse()) | ||||||
| #dependency_relation -= self_dependency | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Subtract the diagonal instead: |
||||||
|
|
||||||
| return dependency_relation | ||||||
|
|
||||||
| write_read = [DependencyRelation(write.insn_id, write.var_name, | ||||||
| get_dependency_relation(write, read), | ||||||
| read.insn_id, "write-read") | ||||||
| for write in write_maps | ||||||
| for read in read_maps | ||||||
| if write.var_name == read.var_name] | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary quadratic complexity? |
||||||
|
|
||||||
| read_write = [DependencyRelation(read.insn_id, read.var_name, | ||||||
| get_dependency_relation(read, write), | ||||||
| write.insn_id, "read-write") | ||||||
| for read in read_maps | ||||||
| for write in write_maps | ||||||
| if read.var_name == write.var_name] | ||||||
|
|
||||||
| write_write = [DependencyRelation(write1.insn_id, write1.var_name, | ||||||
| get_dependency_relation(write1, write2), | ||||||
| write2.insn_id, "write-write") | ||||||
| for write1 in write_maps | ||||||
| for write2 in write_maps | ||||||
| if write1.var_name == write2.var_name] | ||||||
|
|
||||||
| # update depends_on for each instruction | ||||||
| for insn in self.instructions: | ||||||
| for relation in write_read: | ||||||
| if relation.dependent_id == insn.id: | ||||||
| insn.update_depends_on(relation.insn_id) | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| for relation in read_write: | ||||||
| if relation.dependent_id == insn.id: | ||||||
| insn.update_depends_on(relation.insn_id) | ||||||
| for relation in write_write: | ||||||
| if relation.dependent_id == insn.id: | ||||||
| insn.update_depends_on(relation.insn_id) | ||||||
|
|
||||||
| return write_read, read_write, write_write | ||||||
|
|
||||||
| def dep_finder(self): | ||||||
| pass | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
|
|
||||||
| @memoize_method | ||||||
| def recursive_insn_dep_map(self): | ||||||
| """Returns a :class:`dict` mapping an instruction IDs *a* | ||||||
|
|
||||||
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.
Add docstrings, specifying what variables are in the map, what the two ends of the map mean, what naming conventions are used.