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

Precise dependencies compatibility #785

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

a-alveyblanc
Copy link
Contributor

@a-alveyblanc a-alveyblanc commented Jun 15, 2023

  • Adds HappensAfter data structure
  • Adds routine to compute lexicographic order of statements (found in loopy/kernel/dependency.py)
  • Updates InstructionBase to accommodate precise dependency semantics (depends_on -> happens_after)
  • Other changes to remedy the fact that depends_on is now happens_after

Needs:

loopy/kernel/creation.py Outdated Show resolved Hide resolved
loopy/kernel/creation.py Outdated Show resolved Hide resolved
loopy/kernel/dependency.py Outdated Show resolved Hide resolved
loopy/kernel/instruction.py Outdated Show resolved Hide resolved
loopy/kernel/instruction.py Outdated Show resolved Hide resolved
loopy/kernel/instruction.py Outdated Show resolved Hide resolved
loopy/schedule/__init__.py Outdated Show resolved Hide resolved
loopy/schedule/__init__.py Outdated Show resolved Hide resolved
loopy/schedule/__init__.py Outdated Show resolved Hide resolved
test/test_dependencies.py Show resolved Hide resolved
@a-alveyblanc
Copy link
Contributor Author

@inducer This is ready for another look.

@inducer inducer force-pushed the precise-dependencies-compatibility branch 5 times, most recently from bcb356a to 47d3c01 Compare August 6, 2024 20:27
@inducer inducer force-pushed the precise-dependencies-compatibility branch from 47d3c01 to 5c84e95 Compare August 6, 2024 20:33
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

FWIW, I will fix this up in the interest of getting it in soon.

Comment on lines +66 to +91
shared_inames_order_before = [
domain_before.get_dim_name(dim_type.out, idim)
for idim in range(domain_before.dim(dim_type.out))
if domain_before.get_dim_name(dim_type.out, idim)
in shared_inames]

shared_inames_order_after = [
domain_after.get_dim_name(dim_type.out, idim)
for idim in range(domain_after.dim(dim_type.out))
if domain_after.get_dim_name(dim_type.out, idim)
in shared_inames]

assert shared_inames_order_after == shared_inames_order_before
shared_inames_order = shared_inames_order_after
Copy link
Owner

Choose a reason for hiding this comment

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

This snippet here would imply that the order of variables in a set is significant. This is not the case anywhere else in loopy, and I would prefer to not start having it be the case now. This behavior is also not documented.

Copy link
Owner

Choose a reason for hiding this comment

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

So... I tried, but I ran out of time. 🙂 There's a bit of algorithmic work to be done here, and some important algorithmic pieces are in #690. We can discuss in more detail during our meeting Monday.

Rather than hijack this PR for my big pile of unrelated changes, that's now #864, which this depends on. The relevant pieces from #690 are also in #864, so you'll be able to rely on them.

@inducer inducer force-pushed the precise-dependencies-compatibility branch from b475e5a to 507792b Compare August 18, 2024 22:14
@inducer inducer mentioned this pull request Aug 19, 2024
@inducer inducer force-pushed the precise-dependencies-compatibility branch from 507792b to 587f381 Compare August 19, 2024 12:22
@inducer inducer mentioned this pull request Aug 24, 2024
@inducer inducer force-pushed the precise-dependencies-compatibility branch from 587f381 to 1a6548c Compare August 25, 2024 04:41
@inducer inducer marked this pull request as draft August 25, 2024 04:42
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