Skip to content

Commit 9b94d54

Browse files
memmettmemmett
authored
[rocRoller] AIROCROLL-1580 Rename bodyParents to containingAncestors, and augment return value with the containing edge type. (ROCm#6100)
## Motivation This PR renames the KernelGraph helper previously called `bodyParents` to `containingAncestors`, expands its return value to include the containing edge type, and ports the `ControlGraph` tests from gtest to Catch2. ## Technical Details - Replaced `bodyParents(...)` with `containingAncestors(...)` and updated consumers to handle the returned `(node, edge)` pair. - Updated `controlStack(...)` to build from `containingAncestors(...)`. - Moved `ControlGraphTest` from `test/unit` (gtest) to `test/catch` (Catch2) and adjusted CMake sources accordingly.
1 parent a9a0519 commit 9b94d54

8 files changed

Lines changed: 735 additions & 704 deletions

File tree

‎shared/rocroller/lib/include/rocRoller/KernelGraph/NodeSchedulingUtils_impl.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ namespace rocRoller::KernelGraph::NodeScheduling
1515
std::unordered_map<int, std::vector<int>> rv;
1616
for(auto node : theNodes)
1717
{
18-
auto parent = bodyParents(node, graph).take(1).only();
19-
AssertFatal(parent.has_value(), "Node has no body parent", ShowValue(node));
18+
auto parentPair = containingAncestors(node, graph).take(1).only();
19+
AssertFatal(parentPair.has_value(), "Node has no containing ancestor", ShowValue(node));
2020

21-
rv[*parent].push_back(node);
21+
rv[parentPair->first].push_back(node);
2222
}
2323
return rv;
2424
}

‎shared/rocroller/lib/include/rocRoller/KernelGraph/Utils.hpp‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -802,26 +802,30 @@ namespace rocRoller
802802
void removeRedundantBodyEdgesBaselineMethod(KernelGraph& graph);
803803

804804
/**
805-
* Yields all of the nodes that are body parents of `control` in order from the node
806-
* up to the root of the graph.
805+
* Yields all nodes that contain `control` via a non-Sequence edge, in order from
806+
* the immediate parent up to the root of the graph, paired with the containing edge
807+
* connecting the parent to the child (e.g. Body, Else, Initialize, ForLoopIncrement).
807808
*/
808-
Generator<int> bodyParents(int control, KernelGraph const& graph);
809+
Generator<std::pair<int, ControlGraph::ControlEdge>>
810+
containingAncestors(int control, KernelGraph const& graph);
809811

810812
/**
811-
* Yields all of the nodes that are body parents of `control` in order from the node
812-
* up to the root of the graph.
813+
* Yields all nodes that contain `control` via a non-Sequence edge, in order from
814+
* the immediate parent up to the root of the graph, paired with the containing edge
815+
* connecting the parent to the child (e.g. Body, Else, Initialize, ForLoopIncrement).
813816
*/
814-
Generator<int> bodyParents(int control, ControlGraph::ControlGraph const& graph);
817+
Generator<std::pair<int, ControlGraph::ControlEdge>>
818+
containingAncestors(int control, ControlGraph::ControlGraph const& graph);
815819

816820
/**
817-
* Returns all of the nodes that contain `control` with a body
818-
* relationship in order from the root of the graph, including `control` itself.
821+
* Returns all nodes that contain `control` via a non-Sequence edge, in order
822+
* from the root of the graph down to `control` itself.
819823
*/
820824
std::deque<int> controlStack(int control, KernelGraph const& graph);
821825

822826
/**
823-
* Returns all of the nodes that contain `control` with a body
824-
* relationship in order from the root of the graph, including `control` itself.
827+
* Returns all nodes that contain `control` via a non-Sequence edge, in order
828+
* from the root of the graph down to `control` itself.
825829
*/
826830
std::deque<int> controlStack(int control, ControlGraph::ControlGraph const& graph);
827831

‎shared/rocroller/lib/source/KernelGraph/Transformations/ScheduleMultiplyAndLDS.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,8 +431,13 @@ namespace rocRoller::KernelGraph
431431

432432
bool canJoin(KernelGraph const& graph, Chains const& group, Chain const& chain)
433433
{
434-
auto groupParent = bodyParents(group.at(0).at(0), graph).take(1).only();
435-
auto chainParent = bodyParents(chain.at(0), graph).take(1).only();
434+
auto groupParentPair = containingAncestors(group.at(0).at(0), graph).take(1).only();
435+
auto chainParentPair = containingAncestors(chain.at(0), graph).take(1).only();
436+
437+
std::optional<int> groupParent
438+
= groupParentPair ? std::optional(groupParentPair->first) : std::nullopt;
439+
std::optional<int> chainParent
440+
= chainParentPair ? std::optional(chainParentPair->first) : std::nullopt;
436441

437442
auto showParent = [](auto x) -> std::string {
438443
if(x)

‎shared/rocroller/lib/source/KernelGraph/Utils.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,12 +1601,14 @@ namespace rocRoller
16011601
}
16021602
}
16031603

1604-
Generator<int> bodyParents(int control, KernelGraph const& graph)
1604+
Generator<std::pair<int, ControlGraph::ControlEdge>>
1605+
containingAncestors(int control, KernelGraph const& graph)
16051606
{
1606-
return bodyParents(control, graph.control);
1607+
return containingAncestors(control, graph.control);
16071608
}
16081609

1609-
Generator<int> bodyParents(int control, ControlGraph::ControlGraph const& graph)
1610+
Generator<std::pair<int, ControlGraph::ControlEdge>>
1611+
containingAncestors(int control, ControlGraph::ControlGraph const& graph)
16101612
{
16111613
std::unordered_set<int> visitedNodes = {control};
16121614

@@ -1625,10 +1627,10 @@ namespace rocRoller
16251627
AssertFatal(!visitedNodes.contains(node), "Graph contains cycle!");
16261628
visitedNodes.insert(node);
16271629

1628-
auto isContaining
1629-
= !std::holds_alternative<ControlGraph::Sequence>(graph.getEdge(edge));
1630+
auto controlEdge = graph.getEdge(edge);
1631+
auto isContaining = !std::holds_alternative<ControlGraph::Sequence>(controlEdge);
16301632
if(isContaining)
1631-
co_yield node;
1633+
co_yield {node, controlEdge};
16321634

16331635
neighbours = graph.getNeighbours<Graph::Direction::Upstream>(node);
16341636
}
@@ -1639,7 +1641,7 @@ namespace rocRoller
16391641
TIMER(t, "controlStack");
16401642
std::deque<int> rv = {control};
16411643

1642-
for(auto parent : bodyParents(control, graph))
1644+
for(auto [parent, edge] : containingAncestors(control, graph))
16431645
{
16441646
rv.push_front(parent);
16451647
}

‎shared/rocroller/test/catch/CMakeLists.txt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ target_sources(rocroller-tests-catch
4545
"${CMAKE_CURRENT_SOURCE_DIR}/CommandTest.cpp"
4646
"${CMAKE_CURRENT_SOURCE_DIR}/ConnectWorkgroupsTest.cpp"
4747
"${CMAKE_CURRENT_SOURCE_DIR}/ControlFlowRWTracerTest.cpp"
48+
"${CMAKE_CURRENT_SOURCE_DIR}/ControlGraphTest.cpp"
4849
"${CMAKE_CURRENT_SOURCE_DIR}/ConvertPropagationTest.cpp"
4950
"${CMAKE_CURRENT_SOURCE_DIR}/CustomAssertionTest.cpp"
5051
"${CMAKE_CURRENT_SOURCE_DIR}/CustomAssertions.cpp"
@@ -57,8 +58,8 @@ target_sources(rocroller-tests-catch
5758
"${CMAKE_CURRENT_SOURCE_DIR}/ExpressionSerializationTest.cpp"
5859
"${CMAKE_CURRENT_SOURCE_DIR}/ExpressionTest.cpp"
5960
"${CMAKE_CURRENT_SOURCE_DIR}/ExpressionToStringTest.cpp"
60-
"${CMAKE_CURRENT_SOURCE_DIR}/ExpressionTreeOrderingTest.cpp"
6161
"${CMAKE_CURRENT_SOURCE_DIR}/ExpressionTransformationTest.cpp"
62+
"${CMAKE_CURRENT_SOURCE_DIR}/ExpressionTreeOrderingTest.cpp"
6263
"${CMAKE_CURRENT_SOURCE_DIR}/FastArithmeticTest.cpp"
6364
"${CMAKE_CURRENT_SOURCE_DIR}/FastDivisionTransformationTest.cpp"
6465
"${CMAKE_CURRENT_SOURCE_DIR}/FuseLoopsTest.cpp"
@@ -69,20 +70,20 @@ target_sources(rocroller-tests-catch
6970
"${CMAKE_CURRENT_SOURCE_DIR}/HypergraphTest.cpp"
7071
"${CMAKE_CURRENT_SOURCE_DIR}/IdentifyParallelDimensionsTest.cpp"
7172
"${CMAKE_CURRENT_SOURCE_DIR}/InstructionTest.cpp"
72-
"${CMAKE_CURRENT_SOURCE_DIR}/KernelOptionsTest.cpp"
7373
"${CMAKE_CURRENT_SOURCE_DIR}/KernelArgumentsTest.cpp"
7474
"${CMAKE_CURRENT_SOURCE_DIR}/KernelGraphColouringTest.cpp"
7575
"${CMAKE_CURRENT_SOURCE_DIR}/KernelGraphConstraintsTest.cpp"
7676
"${CMAKE_CURRENT_SOURCE_DIR}/KernelGraphRemoveDuplicatesTest.cpp"
7777
"${CMAKE_CURRENT_SOURCE_DIR}/KernelGraphSimplifyTest.cpp"
7878
"${CMAKE_CURRENT_SOURCE_DIR}/KernelGraphUtilsTest.cpp"
79+
"${CMAKE_CURRENT_SOURCE_DIR}/KernelOptionsTest.cpp"
7980
"${CMAKE_CURRENT_SOURCE_DIR}/LDSAllocationTest.cpp"
8081
"${CMAKE_CURRENT_SOURCE_DIR}/LDSModelTest.cpp"
8182
"${CMAKE_CURRENT_SOURCE_DIR}/LargerLDSTest.cpp"
8283
"${CMAKE_CURRENT_SOURCE_DIR}/LoadPackedTest.cpp"
8384
"${CMAKE_CURRENT_SOURCE_DIR}/MEMObserverTest.cpp"
84-
"${CMAKE_CURRENT_SOURCE_DIR}/ModelAddressesTest.cpp"
8585
"${CMAKE_CURRENT_SOURCE_DIR}/MFMACoexecObserverTest.cpp"
86+
"${CMAKE_CURRENT_SOURCE_DIR}/ModelAddressesTest.cpp"
8687
"${CMAKE_CURRENT_SOURCE_DIR}/MultipleLDSAllocationTest.cpp"
8788
"${CMAKE_CURRENT_SOURCE_DIR}/NaryExpressionTest.cpp"
8889
"${CMAKE_CURRENT_SOURCE_DIR}/NodeSchedulingUtilsTest.cpp"

0 commit comments

Comments
 (0)