New dependency finding by a-alveyblanc · Pull Request #683 · inducer/loopy

a-alveyblanc · 2022-09-26T20:11:42Z

No description provided.

- Changed function that generates access relations. (`generate_access_relations()`) - Changed function that generates dependency relations. (`generate_dependency_relations()`) - Added data classes that store information relevant to. - (`RelationInfo`, `AccessRelation(RelationInfo)`, `DependencyRelation(RelationInfo)`

`depends_on`.

…inder

inducer

Thanks for working on this! Looks great, just a few comments below.

inducer · 2022-10-03T20:33:24Z

loopy/kernel/__init__.py

    size: isl.PwAff

+@dataclass(frozen=True)
+class RelationInfo:


Add docstrings, specifying what variables are in the map, what the two ends of the map mean, what naming conventions are used.

inducer · 2022-10-03T20:33:33Z

loopy/kernel/__init__.py

    size: isl.PwAff

+@dataclass(frozen=True)
+class RelationInfo:


Add into documentation.

inducer · 2022-10-03T20:40:41Z

loopy/kernel/__init__.py

+@dataclass(frozen=True)
+class AccessRelation(RelationInfo):
+    access_type: str


Does this want to be an enum?

inducer · 2022-10-03T20:44:55Z

loopy/kernel/__init__.py

+    relation: isl.Map
+
+@dataclass(frozen=True)
+class AccessRelation(RelationInfo):


Should access relations be long-lived?

inducer · 2022-10-03T20:48:08Z

loopy/kernel/__init__.py

+    relation: isl.Map
+
+@dataclass(frozen=True)
+class AccessRelation(RelationInfo):


"has-a happier than is-a"

inducer · 2022-10-03T20:57:28Z

loopy/kernel/__init__.py


    # {{{ dependency wrangling

+    def generate_access_relations(self):


Suggested change

def generate_access_relations(self):

def generate_access_relations(self) -> ...:

inducer · 2022-10-03T21:00:47Z

loopy/kernel/__init__.py

+        for insn in self.instructions:
+            for relation in write_read:
+                if relation.dependent_id == insn.id:
+                    insn.update_depends_on(relation.insn_id)


Suggested change

insn.update_depends_on(relation.insn_id)

insn = insn.copy(depends_on=...)

inducer · 2022-10-03T21:05:19Z

loopy/kernel/__init__.py

+                      for read in read_maps
+                      if write.var_name == read.var_name]


Unnecessary quadratic complexity?

inducer · 2022-10-03T21:06:32Z

loopy/kernel/__init__.py

+    def dep_finder(self):
+        pass


https://github.com/inducer/loopy/pulls/jdsteve2

inducer · 2022-10-03T21:11:29Z

loopy/kernel/__init__.py

+            #self_dependency = x.relation.apply_range(x.relation.reverse())
+
+            dependency_relation = x.relation.apply_range(y.relation.reverse())
+            #dependency_relation -= self_dependency


Subtract the diagonal instead: $\{[i,j] \to [i',j']: i=i' \land j=j'\}$

…ion, and rewrote classes storing information for relations.

…ify_execution_order

…tation of the function

inducer · 2022-10-17T20:49:25Z

loopy/kernel/dependency.py

+    variable_name: str
+    relation: isl.Map
+    dependency_type: DependencyType


Suggested change

variable_name: str

relation: isl.Map

dependency_type: DependencyType

variable_name: Optional[str]

relation: isl.Map

dependency_type: Optional[DependencyType]

(But probably better to make a data-mediated dependency a separate type, maybe a subclass.)

inducer · 2022-10-17T20:50:23Z

loopy/kernel/dependency.py

+    WRITE_READ  = 0
+    READ_WRITE  = 1
+    WRITE_WRITE = 2


Suggested change

WRITE_READ = 0

READ_WRITE = 1

WRITE_WRITE = 2

WRITE_AFTER_READ = 0

READ_AFTER_WRITE = 1

WRITE_AFTER_WRITE = 2

inducer · 2022-10-17T20:51:56Z

loopy/kernel/dependency.py

+from dataclasses import dataclass
+from enum import Enum
+
+class DependencyType(Enum):


I'm unsure whether we'll want this, or whether we'll want all those union'd together.

inducer · 2022-10-17T20:53:03Z

loopy/kernel/dependency.py

+    dependency_type: DependencyType
+
+@dataclass(frozen=True)
+class AccessRelation:


Suggested change

class AccessRelation:

class _AccessRelation:

inducer · 2022-10-17T20:57:28Z

loopy/kernel/dependency.py

+        union_of_dependencies = None
+        for rel in write_read:
+            if union_of_dependencies is None:
+                union_of_dependencies = rel.relation
+            else:
+                union_of_dependencies = union_of_dependencies | rel.relation


functools.reduce

inducer · 2022-10-17T20:58:06Z

loopy/kernel/dependency.py

+        domain: isl.BasicSet = knl.get_inames_domain(insn.within_inames)
+        insn_order: isl.Map = domain.lex_lt_set(domain)
+
+        # v FIXME: there must be a better way


Think about transitive dependencies.

inducer · 2022-10-17T20:58:59Z

loopy/kernel/dependency.py

+
+    for insn in knl.instructions:
+        domain: isl.BasicSet = knl.get_inames_domain(insn.within_inames)
+        insn_order: isl.Map = domain.lex_lt_set(domain)


This shouldn't start from lexicographic, but rather from an already-existing happens-before.

inducer · 2022-10-17T20:59:21Z

loopy/kernel/dependency.py

+    :returns: True or false depending on whether the provided execution order
+    respects the dependencies in the loopy program.
+    """
+    pass


Look at James's prior work.

inducer · 2022-10-17T20:59:39Z

loopy/kernel/dependency.py

+        insn_order = insn_order & union_of_dependencies
+        execution_order = execution_order | frozenset({insn_order})
+
+        return execution_order


Put this into the LoopKernel data structure.

…uced more? Yes (coming soon).

…g dependencies

inducer · 2022-11-07T21:35:11Z

loopy/kernel/dependency.py

+    given kernel transformation respects the data dependencies in a given
+    program.
+
+    .. attribute:: happens_before


Suggested change

.. attribute:: happens_before

.. attribute:: after_id

inducer · 2022-11-07T21:35:36Z

loopy/kernel/dependency.py

+from dataclasses import dataclass
+
+@dataclass(frozen=True)
+class HappensBefore: 


Suggested change

class HappensBefore:

class HappensAfter:

inducer · 2022-11-14T21:31:04Z

Does #704 supersede this?

Temp commit

3c01e0c

a-alveyblanc changed the title ~~Temp commit~~ New dependency finding Sep 26, 2022

a-alveyblanc and others added 4 commits October 1, 2022 21:04

Merge branch 'inducer:main' into dep_finder

1a939a5

Kernel now updates depends_on in line with the description of

e553cce

`depends_on`.

Merge branch 'dep_finder' of github.com:a-alveyblanc/loopy into dep_f…

ed6d03b

…inder

inducer reviewed Oct 3, 2022

View reviewed changes

a-alveyblanc added 9 commits October 9, 2022 22:51

New file created for dependencies, added type annotations, documentat…

a10d3db

…ion, and rewrote classes storing information for relations.

Adjusted alignment so lines of code were not so far past 80 columns

81b26a0

Stub in dep_finder function

09899bf

Add documentation, begin fleshing out

464895f

Add documentation, begin fleshing out generate_execution_order

d947d6b

Implemented draft version of generate_execution_order and stub in ver…

fdb5a01

…ify_execution_order

Last commit does not reflect all changes made in that commit

29ae7c0

Rewrote execution order generation function

0294761

Minor changes to draft of generate_execution_order and to the documen…

6ed1cce

…tation of the function

inducer reviewed Oct 17, 2022

View reviewed changes

a-alveyblanc added 7 commits October 17, 2022 16:42

Removed some things from kernel __init__.py that shouldn't be there

ccfca54

Updated subtraction of diagonal from found dependency relation.

6e47dd0

Replaced dependency union operation with a reduction. Can this be red…

a0fd274

…uced more? Yes (coming soon).

Added various TODOs to mark changes

2569006

Changes in the way dependencies are computed

d35f11b

Using itertools.product instead of doubly nested for loop in computin…

a3a4c15

…g dependencies

Small changes

4d4ebdb

inducer reviewed Nov 7, 2022

View reviewed changes

a-alveyblanc mentioned this pull request Nov 14, 2022

Dependency finding #704

Draft

a-alveyblanc closed this Nov 14, 2022


		# {{{ dependency wrangling

		def generate_access_relations(self):

	def generate_access_relations(self):
	def generate_access_relations(self) -> ...:

	insn.update_depends_on(relation.insn_id)
	insn = insn.copy(depends_on=...)

Conversation

a-alveyblanc commented Sep 26, 2022

Uh oh!

inducer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inducer commented Nov 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants