From e7ab48cb39a820ec84817a89441fb6d943e697ce Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Tue, 3 Mar 2026 15:02:20 -0700 Subject: [PATCH 1/8] [AIEPathfinder] Fix packet_dest-before-packet_source ordering bug (#2583) Split the single if/else-if loop in runOnPacketFlow (AIECreatePathFindFlows.cpp and AIEPathFinder.cpp) into two passes: pass 1 collects the packet_source, pass 2 processes each packet_dest. This removes the implicit assumption that packet_source precedes packet_dest in the block, which MLIR does not guarantee. Tighten PacketFlowOp::verify() to enforce exactly one packet_source and at least one packet_dest, closing the verifier gap that previously allowed zero-source and multi-source flows through. Add a null-check after pass 1 in both transform passes as a defensive backstop against any future verifier bypass. Adds two lit regression tests: - packet_flow_dest_before_source.mlir: verifies correct output for dest-before-source ordering (single dest, multiple dests, keep_pkt_header). - badpacket_flow_source_count.mlir: verifies verifier errors for zero-source, multiple-source, and zero-dest packet flows. Co-Authored-By: Claude Opus 4.6 --- lib/Dialect/AIE/IR/AIEDialect.cpp | 11 +++ .../AIE/Transforms/AIECreatePathFindFlows.cpp | 26 ++++--- lib/Dialect/AIE/Transforms/AIEPathFinder.cpp | 11 ++- .../badpacket_flow_source_count.mlir | 56 +++++++++++++++ .../packet_flow_dest_before_source.mlir | 68 +++++++++++++++++++ 5 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 test/create-packet-flows/badpacket_flow_source_count.mlir create mode 100644 test/create-packet-flows/packet_flow_dest_before_source.mlir diff --git a/lib/Dialect/AIE/IR/AIEDialect.cpp b/lib/Dialect/AIE/IR/AIEDialect.cpp index 13ad039bd28..afa1e19a208 100644 --- a/lib/Dialect/AIE/IR/AIEDialect.cpp +++ b/lib/Dialect/AIE/IR/AIEDialect.cpp @@ -1433,11 +1433,22 @@ LogicalResult PacketFlowOp::verify() { if (body.empty()) return emitOpError("should have non-empty body"); + int numSources = 0, numDests = 0; for (auto &ops : body.front()) { if (!isa(ops)) return ops.emitOpError("cannot be contained in a PacketFlow op"); + if (isa(ops)) + ++numSources; + if (isa(ops)) + ++numDests; } + if (numSources != 1) + return emitOpError("must have exactly one aie.packet_source (got ") + << numSources << ")"; + if (numDests < 1) + return emitOpError("must have at least one aie.packet_dest"); + return success(); } diff --git a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp index 2f04fe7e7fd..24a3a1e4edb 100644 --- a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp +++ b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp @@ -163,8 +163,7 @@ struct ConvertFlowsToInterconnect : OpConversionPattern { } } - LLVM_DEBUG(llvm::dbgs() << tileId << ": " << setting << " | " - << "\n"); + LLVM_DEBUG(llvm::dbgs() << tileId << ": " << setting << " | " << "\n"); } LLVM_DEBUG(llvm::dbgs() @@ -301,12 +300,21 @@ AIEPathfinderPass::runOnPacketFlow(DeviceOp device, OpBuilder &builder, TileOp srcTile, destTile; TileID srcCoords, destCoords; + // Pass 1: extract source (order-independent: dest may appear before source) for (Operation &Op : b.getOperations()) { if (auto pktSource = dyn_cast(Op)) { srcTile = dyn_cast(pktSource.getTile().getDefiningOp()); srcPort = pktSource.port(); srcCoords = {srcTile.colIndex(), srcTile.rowIndex()}; - } else if (auto pktDest = dyn_cast(Op)) { + } + } + if (!srcTile) + return pktFlowOp.emitOpError("has no packet_source; verify() should " + "have caught this"); + + // Pass 2: process each destination using the source extracted above + for (Operation &Op : b.getOperations()) { + if (auto pktDest = dyn_cast(Op)) { destTile = dyn_cast(pktDest.getTile().getDefiningOp()); destPort = pktDest.port(); destCoords = {destTile.colIndex(), destTile.rowIndex()}; @@ -793,14 +801,14 @@ AIEPathfinderPass::runOnPacketFlow(DeviceOp device, OpBuilder &builder, LLVM_DEBUG(llvm::dbgs() << "Port " << tile << " " << stringifyWireBundle(bundle) << " " << channel << '\n'); - LLVM_DEBUG(llvm::dbgs() << "Mask " - << "0x" << llvm::Twine::utohexstr(mask) << '\n'); - LLVM_DEBUG(llvm::dbgs() << "ID " - << "0x" << llvm::Twine::utohexstr(ID) << '\n'); + LLVM_DEBUG(llvm::dbgs() + << "Mask " << "0x" << llvm::Twine::utohexstr(mask) << '\n'); + LLVM_DEBUG(llvm::dbgs() + << "ID " << "0x" << llvm::Twine::utohexstr(ID) << '\n'); for (int i = 0; i < 31; i++) { if ((i & mask) == (ID & mask)) - LLVM_DEBUG(llvm::dbgs() << "matches flow ID " - << "0x" << llvm::Twine::utohexstr(i) << '\n'); + LLVM_DEBUG(llvm::dbgs() << "matches flow ID " << "0x" + << llvm::Twine::utohexstr(i) << '\n'); } } #endif diff --git a/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp b/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp index dcb61e83da1..62b73128c8d 100644 --- a/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp +++ b/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp @@ -40,12 +40,21 @@ LogicalResult DynamicTileAnalysis::runAnalysis(DeviceOp &device) { Port srcPort, dstPort; TileOp srcTile, dstTile; TileID srcCoords, dstCoords; + // Pass 1: extract source (order-independent: dest may appear before source) for (Operation &Op : b.getOperations()) { if (auto pktSource = dyn_cast(Op)) { srcTile = dyn_cast(pktSource.getTile().getDefiningOp()); srcPort = pktSource.port(); srcCoords = {srcTile.colIndex(), srcTile.rowIndex()}; - } else if (auto pktDest = dyn_cast(Op)) { + } + } + if (!srcTile) + return pktFlowOp.emitOpError("has no packet_source; verify() should " + "have caught this"); + + // Pass 2: process each destination using the source extracted above + for (Operation &Op : b.getOperations()) { + if (auto pktDest = dyn_cast(Op)) { dstTile = dyn_cast(pktDest.getTile().getDefiningOp()); dstPort = pktDest.port(); dstCoords = {dstTile.colIndex(), dstTile.rowIndex()}; diff --git a/test/create-packet-flows/badpacket_flow_source_count.mlir b/test/create-packet-flows/badpacket_flow_source_count.mlir new file mode 100644 index 00000000000..19e93230272 --- /dev/null +++ b/test/create-packet-flows/badpacket_flow_source_count.mlir @@ -0,0 +1,56 @@ +//===- badpacket_flow_source_count.mlir ------------------------*- MLIR -*-===// +// +// This file is licensed under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// (c) Copyright 2024 Xilinx Inc. +// +//===----------------------------------------------------------------------===// + +// RUN: aie-opt --verify-diagnostics --split-input-file %s + +// Regression tests for issue #2583 verifier gap: +// PacketFlowOp must have exactly one packet_source and at least one +// packet_dest. Verify that the verifier rejects zero-source and +// multiple-source flows. + +// Test 1: zero sources — must be rejected by verifier. +module { + aie.device(xcvc1902) { + %t11 = aie.tile(1, 1) + // expected-error@+1 {{must have exactly one aie.packet_source (got 0)}} + aie.packet_flow(0x0) { + aie.packet_dest<%t11, Core : 0> + } + } +} + +// ----- + +// Test 2: multiple sources — must be rejected by verifier. +module { + aie.device(xcvc1902) { + %t11 = aie.tile(1, 1) + %t12 = aie.tile(1, 2) + // expected-error@+1 {{must have exactly one aie.packet_source (got 2)}} + aie.packet_flow(0x0) { + aie.packet_source<%t11, West : 0> + aie.packet_source<%t12, West : 0> + aie.packet_dest<%t11, Core : 0> + } + } +} + +// ----- + +// Test 3: zero dests — must be rejected by verifier. +module { + aie.device(xcvc1902) { + %t11 = aie.tile(1, 1) + // expected-error@+1 {{must have at least one aie.packet_dest}} + aie.packet_flow(0x0) { + aie.packet_source<%t11, West : 0> + } + } +} diff --git a/test/create-packet-flows/packet_flow_dest_before_source.mlir b/test/create-packet-flows/packet_flow_dest_before_source.mlir new file mode 100644 index 00000000000..2b270e93315 --- /dev/null +++ b/test/create-packet-flows/packet_flow_dest_before_source.mlir @@ -0,0 +1,68 @@ +//===- packet_flow_dest_before_source.mlir ---------------------*- MLIR -*-===// +// +// This file is licensed under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// (c) Copyright 2024 Xilinx Inc. +// +//===----------------------------------------------------------------------===// + +// RUN: aie-opt --aie-create-pathfinder-flows %s | FileCheck %s + +// Regression test for issue #2583: +// AIEPathfinderPass assumed packet_source always precedes packet_dest in a +// block. MLIR provides no such ordering guarantee. This test verifies that +// placing aie.packet_dest before aie.packet_source produces correct output. + +// Test 1: Single dest before source (basic reproducer from issue #2583) +// CHECK-LABEL: aie.device(xcvc1902) +// CHECK: %[[T11:.*]] = aie.tile(1, 1) +// CHECK: %[[SW11:.*]] = aie.switchbox(%[[T11]]) { +// CHECK: %[[AMSEL0:.*]] = aie.amsel<0> (0) +// CHECK: aie.masterset(Core : 0, %[[AMSEL0]]) +// CHECK: aie.packet_rules(West : 0) { +// CHECK: aie.rule(31, 0, %[[AMSEL0]]) +// CHECK: } +// CHECK: } + +// Test 2: Multiple dests before source (fanout with dest-first ordering) +// CHECK: %[[T12:.*]] = aie.tile(1, 2) +// CHECK: %[[SW12:.*]] = aie.switchbox(%[[T12]]) { +// CHECK: aie.masterset(Core : 0, +// CHECK: aie.masterset(Core : 1, +// CHECK: aie.packet_rules(West : 0) { + +// Test 3: keep_pkt_header with dest-first ordering +// CHECK: %[[T13:.*]] = aie.tile(1, 3) +// CHECK: %[[SW13:.*]] = aie.switchbox(%[[T13]]) { +// CHECK: aie.masterset(Core : 0, +// CHECK: aie.packet_rules(West : 0) { + +module @packet_flow_dest_before_source { + aie.device(xcvc1902) { + + // Test 1: single dest before source + %t11 = aie.tile(1, 1) + aie.packet_flow(0x0) { + aie.packet_dest<%t11, Core : 0> + aie.packet_source<%t11, West : 0> + } + + // Test 2: multiple dests before source + %t12 = aie.tile(1, 2) + aie.packet_flow(0x0) { + aie.packet_dest<%t12, Core : 0> + aie.packet_dest<%t12, Core : 1> + aie.packet_source<%t12, West : 0> + } + + // Test 3: keep_pkt_header with dest-first ordering + %t13 = aie.tile(1, 3) + aie.packet_flow(0x0) { + aie.packet_dest<%t13, Core : 0> + aie.packet_source<%t13, West : 0> + } {keep_pkt_header = true} + + } +} From c479527b685a6c3fcd590cd5009b2ba4079f60b6 Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Tue, 3 Mar 2026 16:13:35 -0700 Subject: [PATCH 2/8] Update lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp index 24a3a1e4edb..b63070092b9 100644 --- a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp +++ b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp @@ -801,10 +801,10 @@ AIEPathfinderPass::runOnPacketFlow(DeviceOp device, OpBuilder &builder, LLVM_DEBUG(llvm::dbgs() << "Port " << tile << " " << stringifyWireBundle(bundle) << " " << channel << '\n'); - LLVM_DEBUG(llvm::dbgs() - << "Mask " << "0x" << llvm::Twine::utohexstr(mask) << '\n'); - LLVM_DEBUG(llvm::dbgs() - << "ID " << "0x" << llvm::Twine::utohexstr(ID) << '\n'); + LLVM_DEBUG(llvm::dbgs() << "Mask " + << "0x" << llvm::Twine::utohexstr(mask) << '\n'); + LLVM_DEBUG(llvm::dbgs() << "ID " + << "0x" << llvm::Twine::utohexstr(ID) << '\n'); for (int i = 0; i < 31; i++) { if ((i & mask) == (ID & mask)) LLVM_DEBUG(llvm::dbgs() << "matches flow ID " << "0x" From e7366d506e43d78d80f55bbdac28323f29ccff1c Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Tue, 3 Mar 2026 16:13:43 -0700 Subject: [PATCH 3/8] Update lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp index b63070092b9..063071ec0e5 100644 --- a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp +++ b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp @@ -807,8 +807,8 @@ AIEPathfinderPass::runOnPacketFlow(DeviceOp device, OpBuilder &builder, << "0x" << llvm::Twine::utohexstr(ID) << '\n'); for (int i = 0; i < 31; i++) { if ((i & mask) == (ID & mask)) - LLVM_DEBUG(llvm::dbgs() << "matches flow ID " << "0x" - << llvm::Twine::utohexstr(i) << '\n'); + LLVM_DEBUG(llvm::dbgs() << "matches flow ID " + << "0x" << llvm::Twine::utohexstr(i) << '\n'); } } #endif From 8dd48b71b6e7f31d3520cdddc014807bea2d9159 Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Tue, 3 Mar 2026 16:14:32 -0700 Subject: [PATCH 4/8] Update lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp index 063071ec0e5..912e47d3235 100644 --- a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp +++ b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp @@ -163,7 +163,8 @@ struct ConvertFlowsToInterconnect : OpConversionPattern { } } - LLVM_DEBUG(llvm::dbgs() << tileId << ": " << setting << " | " << "\n"); + LLVM_DEBUG(llvm::dbgs() << tileId << ": " << setting << " | " + << "\n"); } LLVM_DEBUG(llvm::dbgs() From db2812d45b21407e716e5299d0d421156f7eeae6 Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Tue, 3 Mar 2026 16:20:48 -0700 Subject: [PATCH 5/8] Update lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp index 912e47d3235..5724f1eea15 100644 --- a/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp +++ b/lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp @@ -310,9 +310,7 @@ AIEPathfinderPass::runOnPacketFlow(DeviceOp device, OpBuilder &builder, } } if (!srcTile) - return pktFlowOp.emitOpError("has no packet_source; verify() should " - "have caught this"); - + return pktFlowOp.emitOpError("packet_flow has no packet_source"); // Pass 2: process each destination using the source extracted above for (Operation &Op : b.getOperations()) { if (auto pktDest = dyn_cast(Op)) { From cf8cffa477b9580faf312dc1a2dada69c0eb0139 Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Tue, 3 Mar 2026 16:20:55 -0700 Subject: [PATCH 6/8] Update lib/Dialect/AIE/Transforms/AIEPathFinder.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/Dialect/AIE/Transforms/AIEPathFinder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp b/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp index 62b73128c8d..fbe9a2fb517 100644 --- a/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp +++ b/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp @@ -49,10 +49,10 @@ LogicalResult DynamicTileAnalysis::runAnalysis(DeviceOp &device) { } } if (!srcTile) - return pktFlowOp.emitOpError("has no packet_source; verify() should " - "have caught this"); + return pktFlowOp.emitOpError("packet_flow has no packet_source"); // Pass 2: process each destination using the source extracted above + for (Operation &Op : b.getOperations()) { for (Operation &Op : b.getOperations()) { if (auto pktDest = dyn_cast(Op)) { dstTile = dyn_cast(pktDest.getTile().getDefiningOp()); From 885795fcd3c28caf8709bdf35aae78f5723db466 Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Wed, 4 Mar 2026 09:40:57 -0700 Subject: [PATCH 7/8] Fix duplicate for-loop in Pass 2 of DynamicTileAnalysis::runAnalysis Remove the extra `for (Operation &Op : b.getOperations())` line that was accidentally duplicated when writing the two-pass packet_source/dest fix, causing -Werror=unused-variable and cascading parse failures across all CI. Co-Authored-By: Claude Opus 4.6 --- lib/Dialect/AIE/Transforms/AIEPathFinder.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp b/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp index fbe9a2fb517..bb886474d84 100644 --- a/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp +++ b/lib/Dialect/AIE/Transforms/AIEPathFinder.cpp @@ -52,7 +52,6 @@ LogicalResult DynamicTileAnalysis::runAnalysis(DeviceOp &device) { return pktFlowOp.emitOpError("packet_flow has no packet_source"); // Pass 2: process each destination using the source extracted above - for (Operation &Op : b.getOperations()) { for (Operation &Op : b.getOperations()) { if (auto pktDest = dyn_cast(Op)) { dstTile = dyn_cast(pktDest.getTile().getDefiningOp()); From b57991c9ef57ec7aae465ed3e8f6bfb5bb164f34 Mon Sep 17 00:00:00 2001 From: Erika Hunhoff Date: Wed, 4 Mar 2026 11:01:09 -0700 Subject: [PATCH 8/8] Fix copyright year and company name in new test files Update from '2024 Xilinx Inc.' to '2026 Advanced Micro Devices, Inc.' Co-Authored-By: Claude Opus 4.6 --- test/create-packet-flows/badpacket_flow_source_count.mlir | 2 +- test/create-packet-flows/packet_flow_dest_before_source.mlir | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/create-packet-flows/badpacket_flow_source_count.mlir b/test/create-packet-flows/badpacket_flow_source_count.mlir index 19e93230272..c2126a3ae2b 100644 --- a/test/create-packet-flows/badpacket_flow_source_count.mlir +++ b/test/create-packet-flows/badpacket_flow_source_count.mlir @@ -4,7 +4,7 @@ // See https://llvm.org/LICENSE.txt for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// (c) Copyright 2024 Xilinx Inc. +// (c) Copyright 2026 Advanced Micro Devices, Inc. // //===----------------------------------------------------------------------===// diff --git a/test/create-packet-flows/packet_flow_dest_before_source.mlir b/test/create-packet-flows/packet_flow_dest_before_source.mlir index 2b270e93315..7a32ad3a790 100644 --- a/test/create-packet-flows/packet_flow_dest_before_source.mlir +++ b/test/create-packet-flows/packet_flow_dest_before_source.mlir @@ -4,7 +4,7 @@ // See https://llvm.org/LICENSE.txt for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// (c) Copyright 2024 Xilinx Inc. +// (c) Copyright 2026 Advanced Micro Devices, Inc. // //===----------------------------------------------------------------------===//