fix[dace][next]: Fixes expanding of Not Supported Memlets #2004
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have to expand some Memlets into copy Maps, because there is no suitable call to `cudaMemcpy*`, because we have to set their order right. But this is a gross hack, it should be solved once [PR#1913](GridTools#1913) has been merged, but for that DaCe [PR#1976](spcl/dace#1976) has to be merged. The error was that there was the assumption that at least one of these newly created Maps survived. However, this does not seem to be the case, depending in which order MapFusion is applied. Thus the assert was removed. We now assume that if we do not find a newly created MapEntry that is was integrated into a Map that was already present, we further assume that the Map parameter of that Map were used and that they are correct.
philip-paul-mueller
added a commit
that referenced
this pull request
Jun 11, 2025
If the DaCe GPU code generator encounters a Memlet that can not be expressed as a `cudaMemcpy*()` call, then it converts it to a Map. However, the issue is that these Maps have the wrong iteration order, i.e. wrong memory access pattern and it might not even launch, because of too many blocks in the `y` direction of the compute grid. For this reason GT4Py has to handle these Memlets explicitly. However, [DaCe PR#1976](spcl/dace#1976) changed this slightly and thus GT4Py had to follow. Note, that some of these changes were already introduced by [GT4Py PR#2004](#2004), however, they were made for the original version of the DaCe PR (and the GT4Py PR had to be merged before the DaCe PR was merged). Furthermore, this PR fixes a different issue, also related to the expansion of Memlets, which can be found in [DaCe PR#2033](spcl/dace#2033) (it is not yet merged and currently at commit `19b6bba`). It fixes a bug in how the Memlets are expanded. The DaCe PR also adds the possibility to generate an error instead of slightly converting Memlets into Maps and this PR enables this feature. --------- Co-authored-by: edopao <edoardo.paone@cscs.ch>
stubbiali
pushed a commit
to stubbiali/gt4py
that referenced
this pull request
Aug 19, 2025
…2004) We have to expand some Memlets into copy Maps, because there is no suitable call to `cudaMemcpy*`, because we have to set their order right. But this is a gross hack, it should be solved once [PR#1913](GridTools#1913) has been merged, but for that DaCe [PR#1976](spcl/dace#1976) has to be merged. The error was that there was the assumption that at least one of these newly created Maps survived. However, this does not seem to be the case, depending in which order MapFusion is applied. Thus the assert was removed. We now assume that if we do not find a newly created MapEntry that is was integrated into a Map that was already present, we further assume that the Map parameter of that Map were used and that they are correct.
stubbiali
pushed a commit
to stubbiali/gt4py
that referenced
this pull request
Aug 19, 2025
…#2070) If the DaCe GPU code generator encounters a Memlet that can not be expressed as a `cudaMemcpy*()` call, then it converts it to a Map. However, the issue is that these Maps have the wrong iteration order, i.e. wrong memory access pattern and it might not even launch, because of too many blocks in the `y` direction of the compute grid. For this reason GT4Py has to handle these Memlets explicitly. However, [DaCe PR#1976](spcl/dace#1976) changed this slightly and thus GT4Py had to follow. Note, that some of these changes were already introduced by [GT4Py PR#2004](GridTools#2004), however, they were made for the original version of the DaCe PR (and the GT4Py PR had to be merged before the DaCe PR was merged). Furthermore, this PR fixes a different issue, also related to the expansion of Memlets, which can be found in [DaCe PR#2033](spcl/dace#2033) (it is not yet merged and currently at commit `19b6bba`). It fixes a bug in how the Memlets are expanded. The DaCe PR also adds the possibility to generate an error instead of slightly converting Memlets into Maps and this PR enables this feature. --------- Co-authored-by: edopao <edoardo.paone@cscs.ch>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have to expand some Memlets into copy Maps, because there is no suitable call to
cudaMemcpy*, because we have to set their order right. But this is a gross hack, it should be solved once PR#1913 has been merged, but for that DaCe PR#1976 has to be merged.The error was that there was the assumption that at least one of these newly created Maps survived. However, this does not seem to be the case, depending in which order MapFusion is applied. Thus the assert was removed.
We now assume that if we do not find a newly created MapEntry that is was integrated into a Map that was already present, we further assume that the Map parameter of that Map were used and that they are correct.