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

fix[dace][next]: Propagates view upon changes in strides. #1784

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from

Conversation

philip-paul-mueller
Copy link
Contributor

Some aspects of the stride propagation were not addressed in the original PR.
This PR fixes the behaviour for Views.

edopao and others added 30 commits December 17, 2024 13:29
…timization pipeline when confronted with scans.
However the actuall modifier function is not modified yet.
Which is funny then if you look at the last commit, the number of `not`s in this function was correct.
It seems that we alsohave to handle alias.
It makes thing a bit handler, instead of only looking at the NestedSDFG, we now look at the `(NameOfDataDescriptorInside, NestedSDFG)` pair.
However, it still has some errors.
Before the function had a special mode in which it performed the renaming through the `symbol_mapping`.
However, this made testing a bit harder and so I decided that there should be a flag to disable this.
There are some functioanlity missing, but it is looking good.
…trides.

However, it is not yet fully tested, tehy are on their wa.
It also seems that it inferes with something.
Because a scalar has a shape of `(1,)` but a stride of `()`.
Thus we have first to handle this case.

However, now we are back at the index stuff, let's fix it.
However, it still seems to fail in some cases.
The type is now a bit better estimated.
The type are now extracted from the stuff we get from `free_symbols`.
I realized that allowing this is not very safe.
I also added a test to show that.
@philip-paul-mueller philip-paul-mueller marked this pull request as draft December 20, 2024 10:52
philip-paul-mueller added a commit that referenced this pull request Dec 20, 2024
Added functionality to properly handle changes of strides.

During the implementation of the scan we found that the strides were not handled properly.
Most importantly a change on one level was not propagated into the next levels, i.e. they were still using the old strides.
This PR Solves most of the problems, but there are still some issues that are unsolved:
- Views are not adjusted yet (Fixed in [PR@1784](#1784)).
- It is not properly checked if the symbols of the propagated strides are safe to introduce into the nested SDFG.

The initial functionality of this PR was done by Edoardo Paone (@edopao).

---------
Co-authored-by: edopao <[email protected]>
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