Repository navigation
feat[next-dace]: fold single iteration Map dimensions - #2799
Merged
Merged
Conversation
A Map dimension `i = c:c+1:1` can only ever take the value `c`, but the Memlets inside its scope keep referring to `i` symbolically. `MapFusionVertical` compares producer and consumer subsets with `Range.covers()`, which does not know the Map range, so a producer writing `a[c]` is not recognized as covering a consumer reading `a[i - o]` and a legal fusion is rejected. The new transformation folds the value into the scope and runs before the top level fusion rounds. The dimension itself is kept, unlike DaCe's `TrivialMapElimination` which removes it and thereby prevents the Map from being scheduled on the GPU. On `compute_perturbed_quantities_and_interpolation` this collapses the three surface level kernels into one, 10 -> 9 kernels. It also removes a non-determinism: the same program compiled to 10 or 11 kernels depending on which other variants were compiled in the same session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… folding - drop the `rng[2] == 1` step check: with `start == end` the dimension has a single iteration for any step, so the step carries no information here. - fold with a single `replace_dict()` instead of one `replace()` per parameter. - shorten the note contrasting the transformation with `TrivialMapElimination` to what distinguishes them, the dimension being kept rather than removed. - test the `other_subset` of every Memlet as well, not only the `subset`. - drop the separate fixpoint test: the main test already applies the transformation repeatedly and asserts a single application. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
philip-paul-mueller
approved these changes
Aug 17, 2026
| only_toplevel_maps = dace_properties.Property( | ||
| dtype=bool, | ||
| default=False, | ||
| desc="Only process Maps that are on the top level.", |
Contributor
There was a problem hiding this comment.
Suggested change
| desc="Only process Maps that are on the top level.", | |
| desc="See docs.", |
|
|
||
| # Only apply if a parameter is still referenced, otherwise the transformation | ||
| # would apply again and again on the same Map. | ||
| # NOTE: `include_entry` is needed because the uses are on the out edges of |
Contributor
There was a problem hiding this comment.
The same can be said for include_exit, I would drop the node entirely.
- point the property description at the class docstring - drop the note on `include_entry`, it holds for `include_exit` just as much Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Description
A Map dimension such as
i = c:c+1:1can only ever take the valuec, but the Memlets inside its scope still refer toisymbolically.MapFusionVerticalcompares producer and consumer subsets withRange.covers(), which does not know the Map range, so a producer writinga[c]is not seen to cover a consumer readinga[i - o]even though both denote the same element, and a legal fusion is rejected.TrivialMapDimensionFoldingreplaces the parameter of such a dimension by its value inside the Map scope. On a Map with range1:17, 80over the surface level of an icon4py program:The dimension itself is kept, unlike DaCe's native
TrivialMapEliminationwhich removes it. The transformation runs in_gt_auto_process_top_level_mapsimmediately beforevertical_map_fusion, restricted to top level Maps.On
compute_perturbed_quantities_and_interpolationthe two Maps over the surface level then fuse into one, and nothing else in the program changes. It also removes a source of non-determinism: the same program compiled to a different number of kernels depending on what else was compiled in the same session.ConstantPropagationdoes not cover this case, it propagates symbols assigned on interstate edges and a Map parameter is not one of those.Testing
test_trivial_map_dimension_folding.py: folding rewrites the Memlets while the Map keeps its parameters and range, the SDFG computes the same result before and after, applying repeatedly yields a single application, and a Map without a single iteration dimension is left alone.Requirements
Reviewed on the fork first, at havogt#73.