graph: reject unschedulable graphs at instantiation and stop pruning the caller's graph - #1528
Merged
Merged
Conversation
… graph TestFix1501GraphCompileLoop instantiates and launches a three node graph with a redundant edge twice and expects hipGraphGetNodes and hipGraphGetEdges on the original graph to be unchanged, then instantiates a graph whose only node depends on a node of another graph and a graph with a dependency cycle and expects hipErrorInvalidValue for both. Today the first graph loses its redundant edge on the first launch, and the other two instantiate fine but their launch never returns.
ExtractSubGraphs_ spliced the nodes of every child graph into a local copy of the node list that the scheduler, which is built from the original graph, never saw, so a child graph node still reached CHIPGraphNodeGraph::execute and threw. On the way it added edges between the executable's clones and the child graph's own nodes, read past the end of its node list when the child node was the last node, and, once that read stopped crashing, re-expanded a graph that contains itself forever. Remove it; child graph nodes fail at launch exactly as before.
CHIPGraphExec::compile() builds the launch schedule level by level and had no progress guard: when no remaining node had all of its dependencies placed, because a node depends on a node of another graph or because the dependencies form a cycle and the graph has no root, it pushed empty levels forever and hipGraphLaunch spun at full CPU with unbounded memory growth. A pass that places no node now throws hipErrorInvalidValue, the clone made at instantiation throws hipErrorInvalidValue for a dependency on a node outside the graph instead of mapping it to a null pointer, the path walk of pruneGraph_ throws instead of recursing without end on a cycle, and hipGraphInstantiate compiles the schedule up front so such graphs are rejected there. Since compile() now runs at instantiation and at every launch, it starts from an empty schedule instead of appending to the previous one.
pruneGraph_ removed redundant edges from the nodes of the original hipGraph_t, and compile() runs it on every hipGraphLaunch, so each launch edited the graph the caller still holds and hipGraphGetEdges under reported afterwards. Prune the clone that CHIPGraphExec makes for exactly this purpose instead.
Collaborator
Author
|
/run-aurora-ci |
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
1 participant
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.
Found while running Kokkos::Graph on chipStar.
CHIPGraphExec::compile()had no progress guard, so a graph whose node depends on a node of another graph, or whose dependencies form a cycle, madehipGraphLaunchspin forever with unbounded memory growth; such graphs are now rejected withhipErrorInvalidValue, fromhipGraphInstantiateup front.pruneGraph_()also removed the redundant edges of the caller'shipGraph_ton every launch, sohipGraphGetEdgesunder reported afterwards; it now prunes the executable's own copy.ExtractSubGraphs_is dropped: the scheduler never saw the nodes it spliced in, it read past the end of its node list, and it looped forever on a graph that contains itself. The reproducerTestFix1501GraphCompileLoopcovers all three cases.Fixes #1501