Skip to content

Commit 34afaa9

Browse files
authored
[Router] Fix packet group id assignment (#1335)
Background: The group ID is used inside the router, where only packet flows that share any source or destination port are assigned the same group ID. The router then restricts channel sharing to occur only within the same group, which helps prevent arbiter deadlocks. Consider three packet flows: a, b, and c, where a and c have overlapping source or destination ports, and b is independent. The expected group IDs should be: 0, 1, 0. However, the previous assignment logic was incorrect: `packetGroupId = std::max(packetGroupId, existingId);`, which produced the wrong results: 0, 1, 1. This issue was observed when using out-of-order mode DMA for MatmulThinBias.
1 parent 24dd68d commit 34afaa9

File tree

3 files changed

+69
-23
lines changed

3 files changed

+69
-23
lines changed
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// RUN: iree-opt --amdaie-create-pathfinder-flows %s | FileCheck %s
2+
3+
// Routing test for three packet flows from memtile (0,1) to coretile (0,2).
4+
// Two of the flows target the same destination port, configured as out-of-order mode.
5+
6+
// CHECK-LABEL: aie.device(npu1_4col) {
7+
// CHECK: %[[TILE_0_1:.*]] = aie.tile(0, 1)
8+
// CHECK: %[[SWITCHBOX_0_1:.*]] = aie.switchbox(%[[TILE_0_1]]) {
9+
// CHECK: %[[AMSEL_0:.*]] = aie.amsel<0> (0)
10+
// CHECK: %[[AMSEL_1:.*]] = aie.amsel<1> (0)
11+
// CHECK: %[[MASTERSET_NORTH_0:.*]] = aie.masterset(NORTH : 0, %[[AMSEL_0]])
12+
// CHECK: %[[MASTERSET_NORTH_5:.*]] = aie.masterset(NORTH : 5, %[[AMSEL_1]])
13+
// CHECK: aie.packet_rules(DMA : 1) {
14+
// CHECK: aie.rule(31, 0, %[[AMSEL_0]]) {packet_ids = array<i32: 0>}
15+
// CHECK: }
16+
// CHECK: aie.packet_rules(DMA : 2) {
17+
// CHECK: aie.rule(31, 0, %[[AMSEL_1]]) {packet_ids = array<i32: 0>}
18+
// CHECK: }
19+
// CHECK: aie.packet_rules(DMA : 3) {
20+
// CHECK: aie.rule(31, 0, %[[AMSEL_0]]) {packet_ids = array<i32: 0>}
21+
// CHECK: }
22+
// CHECK: }
23+
// CHECK: %[[TILE_0_2:.*]] = aie.tile(0, 2)
24+
// CHECK: %[[SWITCHBOX_0_2:.*]] = aie.switchbox(%[[TILE_0_2]]) {
25+
// CHECK: %[[AMSEL_0:.*]] = aie.amsel<0> (0)
26+
// CHECK: %[[AMSEL_1:.*]] = aie.amsel<1> (0)
27+
// CHECK: %[[MASTERSET_DMA_0:.*]] = aie.masterset(DMA : 0, %[[AMSEL_0]]) {keep_pkt_header = "true"}
28+
// CHECK: %[[MASTERSET_DMA_1:.*]] = aie.masterset(DMA : 1, %[[AMSEL_1]])
29+
// CHECK: aie.packet_rules(SOUTH : 0) {
30+
// CHECK: aie.rule(31, 0, %[[AMSEL_0]]) {packet_ids = array<i32: 0>}
31+
// CHECK: }
32+
// CHECK: aie.packet_rules(SOUTH : 5) {
33+
// CHECK: aie.rule(31, 0, %[[AMSEL_1]]) {packet_ids = array<i32: 0>}
34+
// CHECK: }
35+
// CHECK: }
36+
// CHECK: }
37+
module {
38+
aie.device(npu1_4col) {
39+
%tile_0_1 = aie.tile(0, 1)
40+
%tile_0_2 = aie.tile(0, 2)
41+
aie.packet_flow(0) {
42+
aie.packet_source<%tile_0_1, DMA : 1>
43+
aie.packet_dest<%tile_0_2, DMA : 0>
44+
} {keep_pkt_header = true}
45+
aie.packet_flow(0) {
46+
aie.packet_source<%tile_0_1, DMA : 2>
47+
aie.packet_dest<%tile_0_2, DMA : 1>
48+
}
49+
aie.packet_flow(0) {
50+
aie.packet_source<%tile_0_1, DMA : 3>
51+
aie.packet_dest<%tile_0_2, DMA : 0>
52+
} {keep_pkt_header = true}
53+
}
54+
}

runtime/src/iree-amd-aie/aie_runtime/iree_aie_router.cc

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -273,32 +273,23 @@ void Router::addFlow(TileLoc srcCoords, Port srcPort, TileLoc dstCoords,
273273
}
274274
}
275275

276-
// Assign a group ID for packet flows
277-
// any overlapping in source/destination will lead to the same group ID
278-
// channel sharing will happen within the same group ID
279-
// for circuit flows, group ID is always -1, and no channel sharing
280-
int packetGroupId = -1;
281-
if (isPacketFlow) {
282-
bool found = false;
276+
// Assign a group ID to packet flows:
277+
// - If the source or any destination overlaps with an existing flow, reuse
278+
// its group ID.
279+
// - Flows sharing the same group ID can share channels.
280+
// - For circuit flows, always assign group ID -1 (no channel sharing).
281+
int32_t packetGroupId = [&]() -> int32_t {
282+
if (!isPacketFlow) return -1;
283283
for (auto &[existingId, src, dsts] : impl->flows) {
284-
if (src.tileLoc == srcCoords && src.port == srcPort) {
285-
packetGroupId = existingId;
286-
found = true;
287-
break;
288-
}
289-
for (auto &dst : dsts) {
290-
if (dst.tileLoc == dstCoords && dst.port == dstPort) {
291-
packetGroupId = existingId;
292-
found = true;
293-
break;
294-
}
284+
if (src.tileLoc == srcCoords && src.port == srcPort) return existingId;
285+
if (llvm::any_of(dsts, [&](const auto &dst) {
286+
return dst.tileLoc == dstCoords && dst.port == dstPort;
287+
})) {
288+
return existingId;
295289
}
296-
packetGroupId = std::max(packetGroupId, existingId);
297-
}
298-
if (!found) {
299-
packetGroupId++;
300290
}
301-
}
291+
return nextAvailablePacketGroupId++;
292+
}();
302293

303294
// If no existing flow was found with this source, create a new flow.
304295
PhysPort srcPhysPort{srcCoords, srcPort, PhysPort::Direction::SRC};

runtime/src/iree-amd-aie/aie_runtime/iree_aie_router.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ struct Router {
140140
std::map<PhysPort, PhysPort> dijkstraShortestPaths(PhysPort src);
141141
std::optional<std::map<PhysPort, SwitchSettings>> findPaths(
142142
int maxIterations = 1000);
143+
int32_t nextAvailablePacketGroupId = 0;
143144
};
144145

145146
// A map from a switchbox output (physical) port to the number of that port.

0 commit comments

Comments
 (0)