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

isl hashes and equality do not consider names #576

Open
inducer opened this issue Mar 11, 2022 · 2 comments
Open

isl hashes and equality do not consider names #576

inducer opened this issue Mar 11, 2022 · 2 comments

Comments

@inducer
Copy link
Owner

inducer commented Mar 11, 2022

cf. inducer/islpy#89

I think this could cause kernels to compare equal that we emphatically wouldn't consider equal. I think what we want is wrapper objects that provides the names and consider them for equality, likely implemented in islpy.

Here's an example of this going wrong:

import loopy as lp

knl1 = lp.make_kernel(
        "{[i,j]: 0<=i<10 and 0<=j<20}",
        "a[i,j] = i+j",
        [lp.GlobalArg("a", shape=(20, 20))])

knl2 = lp.make_kernel(
        "{[j,i]: 0<=j<10 and 0<=i<20}",
        "a[i,j] = i+j",
        [lp.GlobalArg("a", shape=(20, 20))])

assert knl1 == knl2

Runs without complaining on 4a9dabb. 😱

@inducer inducer changed the title Isl hashes and equality do not consider names isl hashes and equality do not consider names Mar 11, 2022
@inducer
Copy link
Owner Author

inducer commented Jan 2, 2023

Looks like the full impact of this was masked by inducer/islpy#102. See inducer/islpy#103 for more context.

@inducer
Copy link
Owner Author

inducer commented Jan 2, 2023

Responding to @kaushikcfd from inducer/islpy#103:

I was thinking I could make a quick PR to loopy which would also add the condition bkt_set.get_var_dict() == set_.get_var_dict(), but the problem is deeper as inducer/loopy#576 points :/. I guess decoupling loopy's ISL usage from relying on dim-names seems the only (practical) option. The other (dirty) option is to make loopy depend on a different islpy that overrides isl_xxx_is_equal.

What to do here is a good question.

My thinking was that we could likely introduce wrapper types around the islpy ones that make names matter for equality. Loopy does not use many types or operations in isl, so this seems somewhat feasible. It would perhaps also allow remedying isl's overly strict Map/Set distinction.

But I'm a ways away from having made up my mind.

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

No branches or pull requests

1 participant