Skip to content

feat[next-dace]: Updated MoveDataflowIntoIfBody#2531

Open
philip-paul-mueller wants to merge 39 commits intoGridTools:mainfrom
philip-paul-mueller:ordered_move_dataflow_into_if
Open

feat[next-dace]: Updated MoveDataflowIntoIfBody#2531
philip-paul-mueller wants to merge 39 commits intoGridTools:mainfrom
philip-paul-mueller:ordered_move_dataflow_into_if

Conversation

@philip-paul-mueller
Copy link
Copy Markdown
Contributor

@philip-paul-mueller philip-paul-mueller commented Mar 13, 2026

This PR updates MoveDataflowIntoIfBody transformation.
It addresses some aspects that were not fully addressed in a previous PR.
The main change is that now the processing is not done in a per connector fashion but is performed in a unified way, i.e. all at once.
Furthermore, the stability was increased, by ordering the nodes to relocate according to their node id.

TODO:

  • Run ICON

Implemented a better sorting mechanism and also handled one case of
empty Memlets, however, not all are handled yet.
@philip-paul-mueller philip-paul-mueller changed the title [feat]: Updated MoveDataflowIntoIfBody feat[next-dace]: Updated MoveDataflowIntoIfBody Mar 25, 2026
@philip-paul-mueller philip-paul-mueller marked this pull request as ready for review March 26, 2026 10:58
Copy link
Copy Markdown
Contributor

@edopao edopao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

assert if_block_spec is not None
enclosing_map = graph.scope_dict()[if_block]
relocatable_connectors, non_relocatable_connectors, connector_usage_location = (
self._partition_if_block(sdfg, if_block) # type: ignore[misc] # Guaranteed to be not None.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use an assert, since it is a prerequisite of this function call, instead of type ignore.

Copy link
Copy Markdown
Contributor Author

@philip-paul-mueller philip-paul-mueller Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the return value is the issue.
Thus one would need:

partion_result = self._partition_if_block()
assert partion_result is not None
el1, el2, el3 = partion_result

However, this looks rely strange to me.
I do that in _has_if_block_relocatable_dataflow() but there it is part of a test that actually uses the None return value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I meant the check on the return value as you posted. Why does it look strange to you?

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.

3 participants