-
Notifications
You must be signed in to change notification settings - Fork 78
Add a transformation to decouple orthogonal dimensions into separate BasicSet #755
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
base: main
Are you sure you want to change the base?
Conversation
e2873d4 to
514bcf7
Compare
40d1906 to
964761e
Compare
964761e to
6d9ab36
Compare
| dom2 = dom2.move_dims(isl.dim_type.param, n_params, dt, pos, 1) | ||
| else: | ||
| dt, pos = dom2.get_var_dict()[iname] | ||
| dom2 = dom2.project_out(dt, pos, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be some checks here that the domains are actually independent, i.e. that the effective domains did not get modified? If nothing else, the documentation should set expectations w.r.t. the correctness guarantees here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If nothing else, the documentation should set expectations w.r.t. the correctness guarantees here.
Point of the..note:: sets the expectations.
| dom2 = dom2.move_dims(isl.dim_type.param, n_params, dt, pos, 1) | ||
| else: | ||
| dt, pos = dom2.get_var_dict()[iname] | ||
| dom2 = dom2.project_out(dt, pos, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not always be semantically equivalent if there are statements within just inames and the remainder of the domain is ever empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Point (2) of the note section in the docs talk about the potential unsoundness of this transformation.
|
|
||
| @for_each_kernel | ||
| def decouple_domain(kernel: LoopKernel, | ||
| inames: Collection[str], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the remaining inames allowed to (parametrically then) depend on inames? (State so in the docs, include a test along those lines.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are simply projected out. We have noted in the docs that this is potentially dependency violating.
No description provided.