Fixed remove_edge_and_dangling_path()#2307
Conversation
… handle cases on the global scope, but some more tests are needed.
tbennun
left a comment
tbennun
left a comment
There was a problem hiding this comment.
Generally looks good, minor comments
| curr_edge.src_conn))) == 0: | ||
| curr_edge.src.remove_out_connector(curr_edge.src_conn) | ||
|
|
||
| # The connector might be collecting. |
There was a problem hiding this comment.
what case is this? I think it is generally disallowed in SDFG syntax
There was a problem hiding this comment.
I actually wish that it would be disallowed but it is permitted and as far as I can tell parts of the toolchain (I suspect ConsolidateEdge) actually create such cases.
The case essentially represents the following:
A: np.ndarray[N, 40] = ...
for i in dace.map[0:N]:
A[i, 0] = some_awesome_boundary_value()
A[i, 39] = another_awesome_boundary_value()
So MapExit will have two incoming edges.
However, they do not have to go to the distinct (in-)connectors of the MapExit node.
In case they go to distinct connectors then there are also two outgoing edges one with subset [0:N, 0] and the other with [0:N, 39].
However, it is also allowed that they go to the same (in-)connector of MapExit.
In that case MapExit will only have one outgoing edge, and it will have a subset of [0:N, 0:40], which grossly overestimate the range that is written.
This is actually currently happening in GT4Py and it also blocks some optimization.
There was a problem hiding this comment.
Yes, there is an exception for passthrough (IN_*/OUT_*) connectors. I was only thinking about tasklets/library nodes/nested SDFGs. Thank you for the clarification
| curr_edge.src_conn))) == 0: | ||
| curr_edge.src.remove_out_connector(curr_edge.src_conn) | ||
|
|
||
| # The connector might be collecting. |
There was a problem hiding this comment.
Yes, there is an exception for passthrough (IN_*/OUT_*) connectors. I was only thinking about tasklets/library nodes/nested SDFGs. Thank you for the clarification
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
2 participants
Fixed
remove_edge_and_dangling_path()such that it also handles the case where the edge is not located inside a Map.