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

Dependency finding #704

Draft
wants to merge 55 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
9f097a3
Sketch out where precise dependencies might go in the IR
inducer Nov 7, 2022
779c2f0
New branch
a-alveyblanc Nov 8, 2022
365781d
Merge branch 'inducer:main' into dependencies-wip
a-alveyblanc Nov 14, 2022
6a58e1d
Added draft of new version of dependency finding
a-alveyblanc Jan 13, 2023
ba3fbf5
Added some documentation
a-alveyblanc Jan 13, 2023
990624b
Started implementation of add_lexicographic_...
a-alveyblanc Jan 20, 2023
93b4126
Update gitignore to ignore DS_STORE
a-alveyblanc Jan 20, 2023
01d3688
Sketch out add_lexicographic_happens_after
inducer Jan 20, 2023
51c460d
Add relation to HappensAfter data structure
a-alveyblanc Jan 21, 2023
f6b75dd
New version of data dependency finding, still WIP
a-alveyblanc Jan 29, 2023
8327c70
added read-after-write, write-after-read, write-after-write dep findi…
a-alveyblanc Feb 4, 2023
999446f
update documentation
a-alveyblanc Feb 6, 2023
f110b30
Added first implementation of dependency finding for non-successive i…
a-alveyblanc Feb 12, 2023
a671762
Minor fixes for some failing tests
a-alveyblanc Feb 14, 2023
34f5b90
Slight refactor to improve readability and remove repeated code
a-alveyblanc Feb 16, 2023
adb924c
Merge branch 'main' into dependencies-wip
a-alveyblanc Feb 16, 2023
a7eb364
More changes to fix failing CI
a-alveyblanc Feb 16, 2023
c23eff4
Some slight changes to fix failing CI and improve readability
a-alveyblanc Feb 16, 2023
1647d5a
More fixes for CI
a-alveyblanc Feb 16, 2023
055d7e3
Add knl.id_to_insn to find instructions from ids instead of looping t…
a-alveyblanc Feb 25, 2023
888291f
Change the way the new happens_after is updated when finding dependen…
a-alveyblanc Feb 28, 2023
b785738
Temporary changes for how to handle cases in which happens_after and …
a-alveyblanc Mar 6, 2023
457cb6f
Revert a change to a test that was not supposed to be changed
a-alveyblanc Mar 6, 2023
096e68b
pushing
a-alveyblanc Mar 20, 2023
1b9f186
Revert "pushing"
a-alveyblanc Mar 21, 2023
5f3a249
Refactor dependency finding code to eliminate bugs.
a-alveyblanc Mar 23, 2023
169e808
Attempt at fixing CI failures due to replacing happens_after with dep…
a-alveyblanc Mar 24, 2023
faa5f23
Remove HappensAfter import from where it never should have been
a-alveyblanc Mar 24, 2023
d3224d1
Minor tweaks. Add a function to print dependency info of a knl
a-alveyblanc Apr 6, 2023
a215511
Refactored code. Much shorter with better access map finding.
a-alveyblanc Apr 10, 2023
72ab7e7
Remove giant todo list from top of file, change printing code
a-alveyblanc Apr 10, 2023
590a8cf
Minor grammatical fix
a-alveyblanc Apr 10, 2023
5208fb6
Minor print function change
a-alveyblanc Apr 10, 2023
cf4b9bd
Implement map_sub_array_ref in AccessMapFinder
a-alveyblanc Apr 11, 2023
faff25c
Read-after-read dependency finding avoided
a-alveyblanc Apr 11, 2023
d967949
Change back to long version of dependency finding. Switch to using pmap
a-alveyblanc Apr 12, 2023
307aa83
Nested array access finding
a-alveyblanc Apr 12, 2023
808cef6
Allow depends_on to be passed as an argument for legacy purposes
a-alveyblanc Apr 17, 2023
8a28e7f
Fix issues found by mypy and pylint
a-alveyblanc Apr 17, 2023
5731025
Fix documentation issues, handle cases where depends_on and happens_a…
a-alveyblanc Apr 18, 2023
e745336
Fix dependency cylce introduced by interaction between add_lexicograp…
a-alveyblanc Apr 21, 2023
b9cc691
Add a possible solution for supporting scalars in dependency finding …
a-alveyblanc Apr 25, 2023
d361167
Pushing [skip ci]
a-alveyblanc Apr 25, 2023
16be472
Precompute coarse-grained dependencies, add traces of transitive depe…
a-alveyblanc May 1, 2023
8e0e384
Use fine-grain dependency information that exists in the kernel when …
a-alveyblanc May 8, 2023
69265b2
Fix failing pylint
a-alveyblanc May 8, 2023
8c42f69
Remove file that was accidentally created
a-alveyblanc May 8, 2023
970feb0
Some unimportant changes
a-alveyblanc Jun 8, 2023
5b98c84
Remove unnecessary tests
a-alveyblanc Jun 9, 2023
2043233
Fix failing doctests
a-alveyblanc Jun 13, 2023
1cea1b3
Remove some (currently) unnecessary changes
a-alveyblanc Jun 13, 2023
9131461
Remove accidentally added files
a-alveyblanc Jun 13, 2023
290c91e
Remove more (temporarily) unnecessary code to prep for compatibility …
a-alveyblanc Jun 13, 2023
7cd7ea0
Fix failing doctest
a-alveyblanc Jun 13, 2023
c1a553a
Merge branch 'inducer:main' into dependencies-wip
a-alveyblanc Sep 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added .DS_Store
Binary file not shown.
31 changes: 3 additions & 28 deletions loopy/kernel/creation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1441,32 +1441,6 @@ def add_assignment(base_name, expr, dtype, additional_inames):
# }}}


# {{{ add_sequential_dependencies

def add_sequential_dependencies(knl):
new_insns = []
prev_insn = None
for insn in knl.instructions:
depon = insn.depends_on
if depon is None:
depon = frozenset()

if prev_insn is not None:
depon = depon | frozenset((prev_insn.id,))

insn = insn.copy(
depends_on=depon,
depends_on_is_final=True)

new_insns.append(insn)

prev_insn = insn

return knl.copy(instructions=new_insns)

# }}}


# {{{ temporary variable creation

def create_temporaries(knl, default_order):
Expand Down Expand Up @@ -1920,7 +1894,8 @@ def apply_single_writer_depencency_heuristic(kernel, warn_if_used=True,
if error_if_used:
raise LoopyError(msg)

insn = insn.copy(depends_on=new_deps)
# insn = insn.copy(depends_on=new_deps)
insn = insn.copy(happens_after=new_deps)
changed = True

new_insns.append(insn)
Expand Down Expand Up @@ -2490,7 +2465,7 @@ def make_function(domains, instructions, kernel_data=None, **kwargs):
check_for_duplicate_insn_ids(knl)

if seq_dependencies:
knl = add_sequential_dependencies(knl)
knl = add_lexicographic_happens_after(knl)

assert len(knl.instructions) == len(inames_to_dup)

Expand Down
112 changes: 112 additions & 0 deletions loopy/kernel/dependency.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import pymbolic.primitives as p

from dataclasses import dataclass
from islpy import Map
from typing import FrozenSet, Optional, List

from loopy import LoopKernel
from loopy import InstructionBase
from loopy.symbolic import WalkMapper

@dataclass(frozen=True)
class HappensAfter:
variable_name: Optional[str]
instances_rel: Optional[Map]

class AccessMapMapper(WalkMapper):
Copy link
Owner

@inducer inducer Jan 13, 2023

Choose a reason for hiding this comment

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

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

"""
TODO Update this documentation so it reflects proper formatting

Used instead of BatchedAccessMapMapper to get single access maps for each
instruction.
"""

def __init__(self, kernel, var_names):
self.kernel = kernel
self._var_names = var_names

from collections import defaultdict
self.access_maps = defaultdict(lambda:
Copy link
Owner

Choose a reason for hiding this comment

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

Document what the keys/values of these mappings are.

defaultdict(lambda:
defaultdict(lambda: None)))

super.__init__()

def map_subscript(self, expr, inames, insn_id):
Copy link
Owner

Choose a reason for hiding this comment

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

Think about type annotation


domain = self.kernel.get_inames_domain(inames)

# why do we need this?
WalkMapper.map_subscript(self, expr, inames)

assert isinstance(expr.aggregate, p.Variable)

if expr.aggregate.name not in self._var_names:
return

arg_name = expr.aggregate.name
subscript = expr.index_tuple

descriptor = self.kernel.get_var_descriptor(arg_name)

from loopy.diagnostic import UnableToDetermineAccessRangeError
from loopy.symbolic import get_access_map

try:
access_map = get_access_map(domain, subscript)
except UnableToDetermineAccessRangeError:
return
Copy link
Owner

Choose a reason for hiding this comment

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

Is this the conservative approach?


if self.access_maps[insn_id][arg_name][inames] is None:
Copy link
Owner

Choose a reason for hiding this comment

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

What about the else case?

self.access_maps[insn_id][arg_name][inames] = access_map

def compute_happens_after(knl: LoopKernel) -> LoopKernel:
"""
TODO Update documentation to reflect the proper format.

Determine dependency relations that exist between instructions. Similar to
apply_single_writer_dependency_heuristic. Extremely rough draft.
"""
writer_map = knl.writer_map()
variables = knl.all_variable_names - knl.inames.keys()

amap = AccessMapMapper(knl, variables)

dep_map = {
insn.id: insn.read_dependency_names() - insn.within_inames
for insn in knl.instructions
}

new_insns = []
for insn in knl.instructions:
Copy link
Owner

Choose a reason for hiding this comment

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

Think about transitive dependencies

Copy link
Owner

Choose a reason for hiding this comment

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

git grep -i transitive

current_insn = insn.id
inames = insn.within_inames

new_happens_after = []
for var in dep_map[insn.id]:
for writer in (writer_map.get(var, set()) - { current_insn }):

# get relation for current instruction and a write instruction
cur_relation = amap.access_maps[current_insn][var][inames]
write_relation = amap.access_maps[writer][var][inames]

# compute the dependency relation
dep_relation = cur_relation.apply_range(write_relation.reverse())

# create the mapping from writer -> (variable, dependency rel'n)
happens_after = HappensAfter(var, dep_relation)
happens_after_mapping = { writer: happens_after }

# add to the new list of dependencies
new_happens_after |= happens_after_mapping

# update happens_after of our current instruction with the mapping
insn = insn.copy(happens_after=new_happens_after)
new_insns.append(insn)

# return the kernel with the new instructions
return knl.copy(instructions=new_insns)

def add_lexicographic_happens_after(knl: LoopKernel) -> None:
Copy link
Owner

Choose a reason for hiding this comment

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

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

pass

Loading