Dependency finding by a-alveyblanc · Pull Request #704 · inducer/loopy

a-alveyblanc · 2022-11-14T21:29:59Z

New draft PR based on PR 683. Major change is that the HappensAfter data structure used to represent statement-level dependency relations has been implemented into the InstructionBase data structure.

inducer · 2023-01-13T19:18:20Z

loopy/kernel/dependency.py

+    variable_name: Optional[str]
+    instances_rel: Optional[Map]
+
+class AccessMapMapper(WalkMapper):


Consider making this a CombineMapper? (In that case, you'd need to define combine)

inducer · 2023-01-13T19:19:03Z

loopy/kernel/dependency.py

+        self._var_names = var_names
+
+        from collections import defaultdict
+        self.access_maps = defaultdict(lambda:


Document what the keys/values of these mappings are.

inducer · 2023-01-13T19:21:35Z

loopy/kernel/dependency.py

+
+        super.__init__()
+
+    def map_subscript(self, expr, inames, insn_id):    


Think about type annotation

inducer · 2023-01-13T19:22:28Z

loopy/kernel/dependency.py

+        try:
+            access_map = get_access_map(domain, subscript)
+        except UnableToDetermineAccessRangeError:
+            return


Is this the conservative approach?

inducer · 2023-01-13T19:23:15Z

loopy/kernel/dependency.py

+        except UnableToDetermineAccessRangeError:
+            return
+
+        if self.access_maps[insn_id][arg_name][inames] is None:


What about the else case?

inducer · 2023-01-13T19:24:01Z

loopy/kernel/dependency.py

+    }
+
+    new_insns = []
+    for insn in knl.instructions:


Think about transitive dependencies

git grep -i transitive

inducer · 2023-01-13T19:29:58Z

loopy/kernel/dependency.py

+    # return the kernel with the new instructions
+    return knl.copy(instructions=new_insns)
+
+def add_lexicographic_happens_after(knl: LoopKernel) -> None:


Lexicographic happens before is the "coarsest" ordering, and the above would be the refinement of that based on data dependencies.

inducer · 2023-01-20T19:49:58Z

loopy/kernel/dependency.py

+            assert shared_inames_order_after == shared_inames_order_before
+            shared_inames_order = shared_inames_order_after


There's a big unresolved question here, in terms of what nesting order we should use for the shared inames. Right now, this uses the axis order in the domains, however we also already do have LoopKernel.loop_priority to indicate nesting order during code generation). If nothing else, this should make sure that the order produced is consistent with that. But there's also the option of using/introducing a different mechanism entirely for this.

@kaushikcfd, got an opinion?

…ng for successive instructions

…nstructions

inducer · 2023-04-10T22:34:17Z

loopy/kernel/dependency.py

+    def map_linear_subscript(self, expr, insn):
+        self.rec(expr.index, insn)


This should probably error.

inducer · 2023-04-10T22:36:14Z

loopy/kernel/dependency.py

+
+    amf = AccessMapFinder(knl)
+    for insn in knl.instructions:
+        amf(insn.assignee, insn)


⚠️ Some instructions (specifically CallInstructions) have more than one assignee.

inducer · 2023-04-10T22:37:21Z

loopy/kernel/dependency.py

+    def get_map(self, insn_id: str, variable_name: str) -> isl.Map:
+        try:
+            return self.access_maps[insn_id][variable_name]
+        except KeyError:


Will this KeyError ever happen?

inducer · 2023-04-10T22:39:02Z

loopy/kernel/dependency.py

+
+    def __init__(self, knl: LoopKernel) -> None:
+        self.kernel = knl
+        self.access_maps = defaultdict(lambda: defaultdict(lambda: None))


Maybe avoid defaultdicts?

inducer · 2023-04-10T22:40:57Z

loopy/kernel/dependency.py

+                domain, subscript, self.kernel.assumptions
+        )
+
+        if self.access_maps[insn.id][arg_name]:


Suggested change

if self.access_maps[insn.id][arg_name]:

if self.access_maps[insn.id][arg_name] is not None:

inducer · 2023-04-10T22:42:27Z

loopy/kernel/dependency.py

+    def map_reduction(self, expr, insn):
+        return WalkMapper.map_reduction(self, expr, insn)
+
+    def map_type_cast(self, expr, inames):


Suggested change

def map_type_cast(self, expr, inames):

def map_type_cast(self, expr, insn):

inducer · 2023-04-10T22:45:12Z

loopy/kernel/dependency.py

+            accessed_by = reader_map.get(variable, set()) | \
+                          writer_map.get(variable, set())


Are you still avoiding read-after-read?

…fter are both specified

…hic_... and apply_single_writer...

…code

…ndency finding in narrow_dependencies [skip ci]

inducer · 2023-04-25T22:20:18Z

loopy/transform/realize_reduction.py

+            happens_after=(
                red_realize_ctx.surrounding_depends_on
                | frozenset([init_id, init_neutral_id])),


The new code suggests that happens_after is still a set?

inducer · 2023-04-25T22:24:20Z

loopy/kernel/dependency.py

+
+@for_each_kernel
+def narrow_dependencies(knl: LoopKernel) -> LoopKernel:
+    """Attempt to relax a strict (lexical) ordering between statements in a


Lexicographic or lexical?

inducer · 2023-04-25T22:25:20Z

loopy/kernel/dependency.py

+
+@for_each_kernel
+def narrow_dependencies(knl: LoopKernel) -> LoopKernel:
+    """Attempt to relax a strict (lexical) ordering between statements in a


Probably not safe to assume that the input ordering is lexicographic.

inducer · 2023-04-25T22:32:31Z

loopy/kernel/dependency.py

+            statements.
+        """
+
+        assert isinstance(insn.id, str) # stop complaints


Turns this into an actual type annotation in InstructionBase.

inducer · 2023-05-01T15:19:08Z

loopy/transform/dependency.py

+    t_sort = compute_topological_order(deps_dag)
+
+    for insn_id in t_sort:
+        transitive_deps[insn_id] = reduce(


transitive_deps can't help but be quadratic in the size of the program.

inducer · 2023-05-01T15:28:00Z

loopy/transform/dependency.py

+                lex_map = insn.happens_after[dependency].instances_rel
+            else:
+                lex_map = dep_map.lex_lt_map(dep_map)


We should use the fine-grain/instance-level dependency info that's already in the kernel at this point.

…computing dependencies

…branch

inducer and others added 3 commits November 7, 2022 17:09

Sketch out where precise dependencies might go in the IR

9f097a3

New branch

779c2f0

Merge branch 'inducer:main' into dependencies-wip

365781d

inducer changed the title ~~New branch for dependency finding~~ Dependency finding Nov 14, 2022

inducer mentioned this pull request Nov 14, 2022

New dependency finding #683

Closed

a-alveyblanc added 2 commits January 13, 2023 12:45

Added draft of new version of dependency finding

6a58e1d

Added some documentation

ba3fbf5

inducer reviewed Jan 13, 2023

View reviewed changes

loopy/kernel/dependency.py Outdated

super.__init__()

def map_subscript(self, expr, inames, insn_id):

Copy link

Owner

inducer Jan 13, 2023

Choose a reason for hiding this comment

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

Think about type annotation

inducer reviewed Jan 13, 2023

View reviewed changes

a-alveyblanc and others added 3 commits January 20, 2023 09:51

Started implementation of add_lexicographic_...

990624b

Update gitignore to ignore DS_STORE

93b4126

Sketch out add_lexicographic_happens_after

01d3688

inducer force-pushed the dependencies-wip branch from cededcd to 01d3688 Compare January 20, 2023 19:44

inducer reviewed Jan 20, 2023

View reviewed changes

a-alveyblanc and others added 11 commits January 21, 2023 16:06

Add relation to HappensAfter data structure

51c460d

New version of data dependency finding, still WIP

f6b75dd

added read-after-write, write-after-read, write-after-write dep findi…

8327c70

…ng for successive instructions

update documentation

999446f

Added first implementation of dependency finding for non-successive i…

f110b30

…nstructions

Minor fixes for some failing tests

a671762

Slight refactor to improve readability and remove repeated code

34f5b90

Merge branch 'main' into dependencies-wip

adb924c

More changes to fix failing CI

a7eb364

Some slight changes to fix failing CI and improve readability

c23eff4

More fixes for CI

1647d5a

a-alveyblanc and others added 5 commits April 6, 2023 11:47

Minor tweaks. Add a function to print dependency info of a knl

d3224d1

Refactored code. Much shorter with better access map finding.

a215511

Remove giant todo list from top of file, change printing code

72ab7e7

Minor grammatical fix

590a8cf

Minor print function change

5208fb6

inducer reviewed Apr 10, 2023

View reviewed changes

a-alveyblanc and others added 10 commits April 11, 2023 08:22

Implement map_sub_array_ref in AccessMapFinder

cf4b9bd

Read-after-read dependency finding avoided

faff25c

Change back to long version of dependency finding. Switch to using pmap

d967949

Nested array access finding

307aa83

Allow depends_on to be passed as an argument for legacy purposes

808cef6

Fix issues found by mypy and pylint

8a28e7f

Fix documentation issues, handle cases where depends_on and happens_a…

5731025

…fter are both specified

Fix dependency cylce introduced by interaction between add_lexicograp…

e745336

…hic_... and apply_single_writer...

Add a possible solution for supporting scalars in dependency finding …

b9cc691

…code

Pushing [skip ci]

d361167

a-alveyblanc force-pushed the dependencies-wip branch from 919dc51 to d361167 Compare April 25, 2023 22:08

Precompute coarse-grained dependencies, add traces of transitive depe…

16be472

…ndency finding in narrow_dependencies [skip ci]

inducer reviewed May 1, 2023

View reviewed changes

a-alveyblanc and others added 11 commits May 8, 2023 16:05

Use fine-grain dependency information that exists in the kernel when …

8e0e384

…computing dependencies

Fix failing pylint

69265b2

Remove file that was accidentally created

8c42f69

Some unimportant changes

970feb0

Remove unnecessary tests

5b98c84

Fix failing doctests

2043233

Remove some (currently) unnecessary changes

1cea1b3

Remove accidentally added files

9131461

Remove more (temporarily) unnecessary code to prep for compatibility …

290c91e

…branch

Fix failing doctest

7cd7ea0

Merge branch 'inducer:main' into dependencies-wip

c1a553a


		super.__init__()

		def map_subscript(self, expr, inames, insn_id):

		assert shared_inames_order_after == shared_inames_order_before
		shared_inames_order = shared_inames_order_after

		def map_linear_subscript(self, expr, insn):
		self.rec(expr.index, insn)

	if self.access_maps[insn.id][arg_name]:
	if self.access_maps[insn.id][arg_name] is not None:

	def map_type_cast(self, expr, inames):
	def map_type_cast(self, expr, insn):

		accessed_by = reader_map.get(variable, set()) \| \
		writer_map.get(variable, set())

Conversation

a-alveyblanc commented Nov 14, 2022

Uh oh!

inducer Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

inducer Jan 13, 2023 •

edited

Loading