feat[next-dace]: Updated MoveDataflowIntoIfBody
#2460
Draft
+195
−19
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
2 tasks
edopao
reviewed
Jan 29, 2026
| # `True` then this order is better. Solve that issue. | ||
| # TODO(phimuell): Because of the limitation that the transformation only works | ||
| # for dataflow that is directly enclosed by a Map, the order in which it is | ||
| # applied matters. Instead we have to run it into a topological order. |
Contributor
There was a problem hiding this comment.
Suggested change
| # applied matters. Instead we have to run it into a topological order. | |
| # applied matters. Instead we should run it into a topological order. |
| that can be relocated, not yet filtered. | ||
| non_relocatable_dataflow: The connectors and their associated dataflow | ||
| that can not be relocated. | ||
| enclosing_map: The limiting node, i.e. the MapEntry of the Map `if_block` |
Contributor
There was a problem hiding this comment.
Suggested change
| enclosing_map: The limiting node, i.e. the MapEntry of the Map `if_block` | |
| enclosing_map: The limiting node, i.e. the MapEntry of the Map where `if_block` |
| for oedge in state.out_edges(reloc_node) | ||
| if oedge.dst is not if_block | ||
| ): | ||
| nodes_proposed_for_reloc.discard(reloc_node) |
Contributor
There was a problem hiding this comment.
Why discard() and not remove()? remove() is more strict because it also checks that it exists, right? and the node has to exists in nodes_proposed_for_reloc
| return branch_nodes | ||
|
|
||
| else: | ||
| # Only some of the incoming nodes will be moved into the `if` |
Contributor
There was a problem hiding this comment.
The first part of the comment is misleading, in my opinion. I would write:
Suggested change
| # Only some of the incoming nodes will be moved into the `if` | |
| # Only some of the incoming nodes could be moved into the `if` |
|
|
||
| else: | ||
| # Only some of the incoming nodes will be moved into the `if` | ||
| # body. This is legal only if the not moved nodes are |
Contributor
There was a problem hiding this comment.
and below:
Suggested change
| # body. This is legal only if the not moved nodes are | |
| # body. It would be legal to relocate the node if all incoming nodes were access nodes, this is why access nodes are not included in 'incoming_nodes'. | |
| # In this case, we cannot relocate the node, so we discard also those 'incoming_nodes' that were candidates for relocation. |
| ``` | ||
| """ | ||
| # This test is temporarily disabled. | ||
| return |
Contributor
There was a problem hiding this comment.
I would use a skip annotation @pytest.mark.skip(reason="")
| state.add_edge(a1, None, top_if_block, "__arg1", dace.Memlet("a1[0]")) | ||
| state.add_edge(b1, None, top_if_block, "__arg2", dace.Memlet("b1[0]")) | ||
| state.add_edge(c1, None, top_if_block, "__cond", dace.Memlet("c1[0]")) | ||
| state.add_edge(top_if_block, "__output", t1, None, dace.Memlet("t1[0]")) |
Contributor
There was a problem hiding this comment.
I tried this test but t1 is not defined.
edopao
changed the title
MoveDataflowIntoIfBodyMoveDataflowIntoIfBody
Contributor
Author
|
No degradation is observed. |
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.
Refines the condition which determines what can be moved into the
ifbody and what not.TODO:
MoveDataflowIntoIfBodytransformation C2SM/icon4py#1015