fix[next][dace]: Better usage of SubgraphContext during SDFG lowering
#2413
Conversation
edopao
changed the title
SubgraphContext during SDFG loweringSubgraphContext during SDFG lowering
This reverts commit 2dbda35.
philip-paul-mueller
left a comment
philip-paul-mueller
left a comment
There was a problem hiding this comment.
An initial review, if you need clarification let me know.
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
| data=target.dc_node.data, subset=target_subset, other_subset=source_subset | ||
| ), | ||
| ) | ||
| if ctx.state.degree(source.dc_node) == 0: |
There was a problem hiding this comment.
What is this node?
Was it created in the (eager) assumption that it is an inout node and it is not?
Because it is also a bit strange considering the creation of the edge before.
There was a problem hiding this comment.
This happens in case of a conditional expression, which ends with a join state containing only an access node to the result, while the dataflow is in the then-else branches.
There was a problem hiding this comment.
For my curiosity, who/where/why was the node put into that state?
There was a problem hiding this comment.
The SDFG will look like this, at this stage, for a simple conditional expression:
The access node to gtir_tmp_4 (and the state) is added by gtir_to_sdfg_primitives.translate_if()
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg_primitives.py
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg_scan.py
Show resolved
Hide resolved
edopao
left a comment
edopao
left a comment
There was a problem hiding this comment.
Thanks for the review comments!
| data=target.dc_node.data, subset=target_subset, other_subset=source_subset | ||
| ), | ||
| ) | ||
| if ctx.state.degree(source.dc_node) == 0: |
There was a problem hiding this comment.
This happens in case of a conditional expression, which ends with a join state containing only an access node to the result, while the dataflow is in the then-else branches.
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg_primitives.py
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg_scan.py
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/lowering/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
philip-paul-mueller
left a comment
philip-paul-mueller
left a comment
There was a problem hiding this comment.
It looks okay, there are some parts I have a bit trouble understanding, especially the motivation behind it, but there should be nothing blocking.
| data=target.dc_node.data, subset=target_subset, other_subset=source_subset | ||
| ), | ||
| ) | ||
| if ctx.state.degree(source.dc_node) == 0: |
There was a problem hiding this comment.
For my curiosity, who/where/why was the node put into that state?
|
Performance looks good: We wait for the icon4py PR#896 to be merged, because we need to remove a write-write pattern which dace state fusion cannot handle (see spcl/dace#2256). This PR reveals the issue, because of non-deterministic code generation, by introducing a new name convention of transient arrays and map nodes. |
2 participants
Change the visitor class so that we can generate unique labels for temporaries/maps/tasklets across all nested SDFG.