loopy icon indicating copy to clipboard operation
loopy copied to clipboard

isl hashes and equality do not consider names

Open inducer opened this issue 3 years ago • 3 comments

cf. https://github.com/inducer/islpy/issues/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 4a9dabbc. :scream:

inducer avatar Mar 11 '22 02:03 inducer

Looks like the full impact of this was masked by https://github.com/inducer/islpy/issues/102. See https://github.com/inducer/islpy/pull/103 for more context.

inducer avatar Jan 02 '23 01:01 inducer

Responding to @kaushikcfd from https://github.com/inducer/islpy/pull/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.

inducer avatar Jan 02 '23 07:01 inducer