Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Router] Fix conflicted packet_rule entries #1087

Merged
merged 6 commits into from
Feb 10, 2025
Merged

[Router] Fix conflicted packet_rule entries #1087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
29e67c7
init commit
Yu-Zhewen Feb 7, 2025
451d47a
Merge branch 'main' into zhewen_packet_rules
Yu-Zhewen Feb 7, 2025
f015ed9
check the order of rules
Yu-Zhewen Feb 7, 2025
c80566c
use uint32_t for packet ids
Yu-Zhewen Feb 10, 2025
09255b8
a better mask construction
Yu-Zhewen Feb 10, 2025
0d270e0
Merge branch 'main' into zhewen_packet_rules
Yu-Zhewen Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 44 additions & 25 deletions compiler/plugins/target/AMD-AIE/aie/AMDAIECreatePathFindFlows.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,25 @@ ConnectOp getOrCreateConnect(OpBuilder &builder, Operation *parentOp,
srcChannel, destBundle, destChannel);
}

PacketRulesOp getOrCreatePacketRules(OpBuilder &builder, SwitchboxOp &swboxOp,
StrmSwPortType bundle, int channel) {
Block &b = swboxOp.getConnections().front();
OpBuilder::InsertionGuard g(builder);
builder.setInsertionPoint(b.getTerminator());
for (auto packetRules : swboxOp.getOps<PacketRulesOp>()) {
builder.setInsertionPointAfter(packetRules);
if (packetRules.getSourceBundle() == bundle &&
packetRules.getSourceChannel() == channel) {
return packetRules;
}
}
auto packetRules =
builder.create<PacketRulesOp>(builder.getUnknownLoc(), bundle, channel);
PacketRulesOp::ensureTerminator(packetRules.getRules(), builder,
builder.getUnknownLoc());
return packetRules;
}

struct ConvertFlowsToInterconnect : OpConversionPattern<FlowOp> {
using OpConversionPattern::OpConversionPattern;
const std::map<PathEndPoint, SwitchSettings> flowSolutions;
Expand Down Expand Up @@ -276,8 +295,8 @@ LogicalResult runOnPacketFlow(
}
auto [masterSets, slaveAMSels] = maybeRoutingConfiguration.value();

auto [slaveGroups, slaveMasks] =
emitSlaveGroupsAndMasksRoutingConfig(slavePorts, packetFlows);
auto [slaveGroups, slaveMasks] = emitSlaveGroupsAndMasksRoutingConfig(
slavePorts, packetFlows, deviceModel.getPacketIdMaskWidth());

// Realize the routes in MLIR
for (auto &[tileLoc, tileOp] : tiles) {
Expand Down Expand Up @@ -340,37 +359,37 @@ LogicalResult runOnPacketFlow(
msOp->setAttr("keep_pkt_header", pktFlowAttrs);
}

// Generate the packet rules
// Generate the packet rules.
uint32_t numPacketRuleSlots =
deviceModel.getNumPacketRuleSlots(tileLoc.col, tileLoc.row);
DenseMap<Port, PacketRulesOp> slaveRules;
for (std::vector<PhysPortAndID> group : slaveGroups) {
builder.setInsertionPoint(b.getTerminator());
PhysPortAndID physPortAndId = group.front();
PhysPort physPort = physPortAndId.physPort;
for (auto &[physPort, groups] : slaveGroups) {
if (tileLoc != physPort.tileLoc) continue;
Port slave = physPort.port;
int mask = slaveMasks[physPortAndId];
int ID = physPortAndId.id & mask;
for (std::set<uint32_t> &group : groups) {
PhysPortAndID physPortAndId(physPort, *group.begin());
uint32_t mask = slaveMasks[physPortAndId];
uint32_t maskedId = physPortAndId.id & mask;

#ifndef NDEBUG
// Verify that we actually map all the ID's correctly.
for (PhysPortAndID _slave : group) assert((_slave.id & mask) == ID);
// Verify that we actually map all the ID's correctly.
for (uint32_t _pktId : group) assert((_pktId & mask) == maskedId);
#endif

Value amsel = amselOps[slaveAMSels[physPortAndId]];
PacketRulesOp packetrules;
if (slaveRules.count(slave) == 0) {
packetrules = builder.create<PacketRulesOp>(
builder.getUnknownLoc(), (slave.bundle), slave.channel);
PacketRulesOp::ensureTerminator(packetrules.getRules(), builder,
builder.getUnknownLoc());
slaveRules[slave] = packetrules;
} else {
packetrules = slaveRules[slave];
Value amsel = amselOps[slaveAMSels[physPortAndId]];
PacketRulesOp packetrules =
getOrCreatePacketRules(builder, swbox, slave.bundle, slave.channel);
// Ensure the number of packet rules does not exceed the allowed slots.
if (groups.size() > numPacketRuleSlots) {
return packetrules.emitOpError()
<< "Exceeded packet rule limit. Allowed: "
<< numPacketRuleSlots << " Required: " << groups.size();
}
Block &rules = packetrules.getRules().front();
builder.setInsertionPoint(rules.getTerminator());
builder.create<PacketRuleOp>(builder.getUnknownLoc(), mask, maskedId,
amsel);
}

Block &rules = packetrules.getRules().front();
builder.setInsertionPoint(rules.getTerminator());
builder.create<PacketRuleOp>(builder.getUnknownLoc(), mask, ID, amsel);
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/plugins/target/AMD-AIE/aie/test/test_create_packet_flows0.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ module @test_create_packet_flows0 {
// CHECK: %[[VAL_4:.*]] = aie.masterset(CORE : 0, %[[VAL_2:.*]])
// CHECK: %[[VAL_5:.*]] = aie.masterset(CORE : 1, %[[VAL_3:.*]])
// CHECK: aie.packet_rules(WEST : 0) {
// CHECK-DAG: aie.rule(31, 0, %[[VAL_2]])
// CHECK-DAG: aie.rule(31, 1, %[[VAL_3]])
// CHECK: aie.rule(31, 0, %[[VAL_2]])
// CHECK: aie.rule(31, 1, %[[VAL_3]])
// CHECK: }
// CHECK: }
// CHECK: }
Expand Down
4 changes: 2 additions & 2 deletions compiler/plugins/target/AMD-AIE/aie/test/test_create_packet_flows2.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
// CHECK: %[[VAL_2:.*]] = aie.masterset(CORE : 0, %[[VAL_0]])
// CHECK: %[[VAL_3:.*]] = aie.masterset(CORE : 1, %[[VAL_0]], %[[VAL_1]])
// CHECK: aie.packet_rules(WEST : 0) {
// CHECK-DAG: aie.rule(31, 0, %[[VAL_0]])
// CHECK-DAG: aie.rule(31, 1, %[[VAL_1]])
// CHECK: aie.rule(31, 0, %[[VAL_0]])
// CHECK: aie.rule(31, 1, %[[VAL_1]])
// CHECK: }
// CHECK: }
// CHECK: }
Expand Down
6 changes: 3 additions & 3 deletions compiler/plugins/target/AMD-AIE/aie/test/test_create_packet_flows4.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ module @test_create_packet_flows4 {
// CHECK: %[[VAL_4:.*]] = aie.masterset(CORE : 0, %[[VAL_2]])
// CHECK: %[[VAL_5:.*]] = aie.masterset(CORE : 1, %[[VAL_3]])
// CHECK: aie.packet_rules(WEST : 0) {
// CHECK-DAG: aie.rule(31, 0, %[[VAL_2]])
// CHECK-DAG: aie.rule(31, 1, %[[VAL_3]])
// CHECK: aie.rule(31, 0, %[[VAL_2]])
// CHECK: aie.rule(31, 1, %[[VAL_3]])
// CHECK: }
// CHECK: aie.packet_rules(WEST : 1) {
// CHECK-DAG: aie.rule(31, 0, %[[VAL_3]])
// CHECK: aie.rule(31, 0, %[[VAL_3]])
// CHECK: }
// CHECK: }
// CHECK: }
Expand Down
4 changes: 2 additions & 2 deletions compiler/plugins/target/AMD-AIE/aie/test/test_create_packet_flows5.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
// CHECK: %[[VAL_2:.*]] = aie.amsel<0> (0)
// CHECK: %[[VAL_3:.*]] = aie.masterset(CORE : 0, %[[VAL_2]])
// CHECK: aie.packet_rules(WEST : 0) {
// CHECK-DAG: aie.rule(30, 0, %[[VAL_2]])
// CHECK: aie.rule(30, 0, %[[VAL_2]])
// CHECK: }
// CHECK: aie.packet_rules(WEST : 1) {
// CHECK-DAG: aie.rule(31, 2, %[[VAL_2]])
// CHECK: aie.rule(31, 2, %[[VAL_2]])
// CHECK: }
// CHECK: }
// CHECK: }
Expand Down
20 changes: 11 additions & 9 deletions compiler/plugins/target/AMD-AIE/aie/test/test_pktflow_weight_pusher.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@
// CHECK: %[[VAL_26:.*]] = aie.masterset(DMA : 1, %[[VAL_25]])
// CHECK: %[[VAL_27:.*]] = aie.masterset(SOUTH : 1, %[[VAL_24]])
// CHECK: aie.packet_rules(NORTH : 2) {
// CHECK: aie.rule(27, 8, %[[VAL_24]])
// CHECK: aie.rule(31, 13, %[[VAL_25]])
// CHECK: aie.rule(27, 8, %[[VAL_24]])
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_2_4:.*]] = aie.tile(2, 4)
Expand All @@ -107,8 +107,8 @@
// CHECK: %[[VAL_34:.*]] = aie.masterset(DMA : 1, %[[VAL_33]])
// CHECK: %[[VAL_35:.*]] = aie.masterset(WEST : 1, %[[VAL_32]])
// CHECK: aie.packet_rules(NORTH : 1) {
// CHECK: aie.rule(28, 0, %[[VAL_32]])
// CHECK: aie.rule(31, 6, %[[VAL_33]])
// CHECK: aie.rule(28, 0, %[[VAL_32]])
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_4_4:.*]] = aie.tile(4, 4)
Expand All @@ -134,8 +134,8 @@
// CHECK: %[[VAL_44:.*]] = aie.masterset(DMA : 1, %[[VAL_43]])
// CHECK: %[[VAL_45:.*]] = aie.masterset(SOUTH : 2, %[[VAL_42]])
// CHECK: aie.packet_rules(EAST : 1) {
// CHECK: aie.rule(26, 8, %[[VAL_42]])
// CHECK: aie.rule(31, 14, %[[VAL_43]])
// CHECK: aie.rule(26, 8, %[[VAL_42]])
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_2_5:.*]] = aie.tile(2, 5)
Expand All @@ -155,9 +155,9 @@
// CHECK: %[[VAL_52:.*]] = aie.masterset(SOUTH : 1, %[[VAL_48]])
// CHECK: %[[VAL_53:.*]] = aie.masterset(WEST : 1, %[[VAL_49]])
// CHECK: aie.packet_rules(EAST : 2) {
// CHECK: aie.rule(24, 0, %[[VAL_48]])
// CHECK: aie.rule(31, 3, %[[VAL_49]])
// CHECK: aie.rule(31, 7, %[[VAL_50]])
// CHECK: aie.rule(24, 0, %[[VAL_48]])
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_4_5:.*]] = aie.tile(4, 5)
Expand All @@ -171,12 +171,12 @@
// CHECK: %[[VAL_60:.*]] = aie.masterset(SOUTH : 3, %[[VAL_55]])
// CHECK: %[[VAL_61:.*]] = aie.masterset(WEST : 2, %[[VAL_54]])
// CHECK: aie.packet_rules(EAST : 2) {
// CHECK: aie.rule(24, 0, %[[VAL_54]])
// CHECK: aie.rule(31, 5, %[[VAL_55]])
// CHECK: aie.rule(24, 0, %[[VAL_54]])
// CHECK: }
// CHECK: aie.packet_rules(EAST : 3) {
// CHECK: aie.rule(28, 8, %[[VAL_56]])
// CHECK: aie.rule(31, 11, %[[VAL_57]])
// CHECK: aie.rule(28, 8, %[[VAL_56]])
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_5_5:.*]] = aie.tile(5, 5)
Expand All @@ -191,8 +191,8 @@
// CHECK: aie.rule(24, 0, %[[VAL_62]])
// CHECK: }
// CHECK: aie.packet_rules(EAST : 3) {
// CHECK: aie.rule(28, 8, %[[VAL_63]])
// CHECK: aie.rule(31, 15, %[[VAL_64]])
// CHECK: aie.rule(28, 8, %[[VAL_63]])
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_6_5:.*]] = aie.tile(6, 5)
Expand All @@ -210,8 +210,10 @@
// CHECK: aie.rule(24, 0, %[[VAL_69]])
// CHECK: }
// CHECK: aie.packet_rules(EAST : 0) {
// CHECK: aie.rule(24, 8, %[[VAL_70]])
// CHECK: aie.rule(24, 8, %[[VAL_71]])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good example to show the issue. Previously, both rules here share the exact same condition, which is incorrect and causes data to go through the wrong arbiter.

// CHECK: aie.rule(31, 9, %3)
// CHECK: aie.rule(31, 10, %3)
// CHECK: aie.rule(27, 11, %3)
// CHECK: aie.rule(24, 8, %2)
// CHECK: }
// CHECK: }
// CHECK: %[[TILE_7_5:.*]] = aie.tile(7, 5)
Expand Down
Loading
Loading