[AIEPlacer] Address PR #3042 review feedback

Reuse, simplification, and broader peer-op support driven by Copilot
review comments and self-review:

* Reuse: delegate cascade direction check to `targetModel->isSouth` /
  `isEast` (the same predicates `AIELowerCascadeFlowsPass` uses), so
  placer and lowering agree by construction. Removes the inline
  arithmetic and its inverted-feeling comments.

* Data structure: replace `pair<LogicalTileOp, bool>` adjacency entries
  with a single edge list (`{src, dst}`) plus per-tile index. The
  `thisIsCascadeDst` boolean falls out — direction is derived at probe
  time by comparing op identity to edge endpoints. Move
  `CascadeAdjacency` into `SequentialPlacer`'s private section.

* Mixed `aie.tile` / `aie.logical_tile` cascade flows: edges now hold
  `TileLike`. A `resolvePeerPosition` helper uniformly handles three
  peer coord sources — placed `LogicalTileOp` (in `result`), `TileOp`
  (always pinned), and pre-pinned `LogicalTileOp` (via
  `tryGetCol`/`tryGetRow`). `tileToEdges` only indexes `LogicalTileOp`
  endpoints since `TileOp`s are never visited by the placer.

* Iteration-order brittleness: the `tryGetCol`/`tryGetRow` fallback
  fully subsumes the earlier stable-partition workaround — an
  unconstrained tile iterated before its pinned peer now resolves the
  peer's pin coords directly during candidate filtering. Partition
  removed.

* Error messages: each cascade-induced placement failure now attaches
  one MLIR diagnostic note per placed peer, naming role
  (source/destination) and physical position. Test comment fixed to
  match `AIELowerCascadeFlows`'s own wording (one row North or one
  column West, rows-up convention noted).

* Tests: add `cascade_chain_pinned_middle` (mid-chain anchor),
  `cascade_hybrid_tileop_peer` (positive `aie.tile` peer),
  `cascade_partial_constraint_unsatisfiable` (candidate-filter
  rejection path with new note format), `cascade_hybrid_unsatisfiable`
  (negative `aie.tile` peer with conflicting partial constraint).

All 116 tests pass in `test/place-tiles` + `test/dialect/AIE` (1
pre-existing XFAIL). Downstream mlir-air rebuilds clean against the
new public API (`CascadeAdjacency` is now private to
`SequentialPlacer` so the change is encapsulated anyway).

Author: erwei-xilinx

include/aie/Dialect/AIE/Transforms/AIEPlacer.h

@@ -22,15 +22,6 @@ enum class PlacerType { SequentialPlacer };
 // maps logical tile operations to physical coordinates
 using PlacementResult = llvm::DenseMap<mlir::Operation *, TileID>;
 
-// For each LogicalTileOp involved in a cascade flow, the set of (peer,
-// thisIsCascadeDst) tuples. `thisIsCascadeDst==true` means this tile is the
-// destination of a cascade_flow whose source is `peer`; the lowered cascade
-// requires `peer` to be one row higher (north) or one column to the left
-// (west) of this tile. `thisIsCascadeDst==false` is the reverse.
-using CascadeAdjacency =
-    llvm::DenseMap<mlir::Operation *,
-                   llvm::SmallVector<std::pair<LogicalTileOp, bool>, 2>>;
-
 // Track available tiles and resource usage
 struct TileAvailability {
   std::vector<TileID> compTiles;
@@ -123,15 +114,26 @@ class SequentialPlacer : public Placer {
       llvm::SmallVector<ObjectFifoCreateOp> &objectFifos,
       llvm::SmallVector<ObjectFifoLinkOp> &objectFifoLinks);
 
-  void buildCascadeAdjacency(llvm::ArrayRef<CascadeFlowOp> cascadeFlows,
-                             CascadeAdjacency &adjacency);
+  // `tileToEdges` indexes into `edges` only for `LogicalTileOp` endpoints
+  // (the ones the placer visits); `TileOp` peers contribute coords via
+  // `TileLike::tryGetCol`/`tryGetRow`.
+  struct CascadeAdjacency {
+    llvm::SmallVector<std::pair<TileLike, TileLike>, 4> edges;
+    llvm::DenseMap<mlir::Operation *, llvm::SmallVector<unsigned, 2>>
+        tileToEdges;
+  };
+
+  CascadeAdjacency
+  buildCascadeAdjacency(llvm::ArrayRef<CascadeFlowOp> cascadeFlows);
 
-  // Returns true iff placing `logicalTile` at `candidate` would satisfy every
-  // cascade adjacency constraint relative to peers that are already placed in
-  // `result`. Unplaced peers impose no constraint at this point — they will
-  // be checked when they themselves get placed.
+  // Unplaced peers without pin coords defer; symmetric predicate makes one
+  // check per endpoint sufficient.
   bool satisfiesCascadeAdjacency(LogicalTileOp logicalTile, TileID candidate,
                                  const CascadeAdjacency &adjacency) const;
+
+  void attachCascadePeerNotes(mlir::InFlightDiagnostic &diag,
+                              LogicalTileOp logicalTile,
+                              const CascadeAdjacency &adjacency) const;
 };
 
 } // namespace xilinx::AIE

lib/Dialect/AIE/Transforms/AIEPlacer.cpp

@@ -138,8 +138,7 @@ LogicalResult SequentialPlacer::place(DeviceOp device) {
   auto channelRequirements =
       buildChannelRequirements(objectFifos, objectFifoLinks);
 
-  CascadeAdjacency cascadeAdjacency;
-  buildCascadeAdjacency(cascadeFlows, cascadeAdjacency);
+  auto cascadeAdjacency = buildCascadeAdjacency(cascadeFlows);
 
   // Phase 3: Place constrained tiles then compute tiles
   size_t nextCompIdx = 0;
@@ -149,10 +148,13 @@ LogicalResult SequentialPlacer::place(DeviceOp device) {
     auto row = logicalTile.tryGetRow();
     if (col && row) {
       TileID tile{*col, *row};
-      if (!satisfiesCascadeAdjacency(logicalTile, tile, cascadeAdjacency))
-        return logicalTile.emitError()
-               << "tile (" << tile.col << ", " << tile.row
-               << ") violates cascade adjacency with an already-placed peer";
+      if (!satisfiesCascadeAdjacency(logicalTile, tile, cascadeAdjacency)) {
+        auto diag = logicalTile.emitError()
+                    << "tile (" << tile.col << ", " << tile.row
+                    << ") violates cascade adjacency";
+        attachCascadePeerNotes(diag, logicalTile, cascadeAdjacency);
+        return failure();
+      }
       if (failed(validateAndUpdateChannelUsage(logicalTile, tile,
                                                channelRequirements, true)))
         return failure();
@@ -190,17 +192,21 @@ LogicalResult SequentialPlacer::place(DeviceOp device) {
       }
 
       if (!placement) {
-        bool hasCascade = cascadeAdjacency.count(logicalTile.getOperation());
+        bool hasCascade =
+            cascadeAdjacency.tileToEdges.count(logicalTile.getOperation());
+        InFlightDiagnostic diag = logicalTile.emitError();
         if (col || row) {
-          return logicalTile.emitError()
-                 << "no compute tile available matching constraint ("
-                 << (col ? std::to_string(*col) : "?") << ", "
-                 << (row ? std::to_string(*row) : "?") << ")"
-                 << (hasCascade ? " and cascade adjacency" : "");
-        }
-        return logicalTile.emitError()
-               << "no available compute tiles for placement"
+          diag << "no compute tile available matching constraint ("
+               << (col ? std::to_string(*col) : "?") << ", "
+               << (row ? std::to_string(*row) : "?") << ")"
+               << (hasCascade ? " and cascade adjacency" : "");
+        } else {
+          diag << "no available compute tiles for placement"
                << (hasCascade ? " (cascade adjacency unsatisfiable)" : "");
+        }
+        if (hasCascade)
+          attachCascadePeerNotes(diag, logicalTile, cascadeAdjacency);
+        return failure();
       }
 
       if (failed(validateAndUpdateChannelUsage(logicalTile, *placement,
@@ -488,52 +494,75 @@ SequentialPlacer::buildChannelRequirements(
   return channelRequirements;
 }
 
-void SequentialPlacer::buildCascadeAdjacency(
-    ArrayRef<CascadeFlowOp> cascadeFlows, CascadeAdjacency &adjacency) {
+static std::optional<TileID>
+resolvePeerPosition(TileLike peer, const PlacementResult &placed) {
+  auto it = placed.find(peer.getOperation());
+  if (it != placed.end())
+    return it->second;
+  auto col = peer.tryGetCol();
+  auto row = peer.tryGetRow();
+  if (col && row)
+    return TileID{*col, *row};
+  return std::nullopt;
+}
+
+SequentialPlacer::CascadeAdjacency
+SequentialPlacer::buildCascadeAdjacency(ArrayRef<CascadeFlowOp> cascadeFlows) {
+  CascadeAdjacency adjacency;
   for (auto cf : cascadeFlows) {
-    auto src =
-        dyn_cast_or_null<LogicalTileOp>(cf.getSourceTile().getDefiningOp());
-    auto dst =
-        dyn_cast_or_null<LogicalTileOp>(cf.getDestTile().getDefiningOp());
+    TileLike src = cf.getSourceTileLike();
+    TileLike dst = cf.getDestTileLike();
     if (!src || !dst)
       continue;
-    // src records dst as a peer for which it is the cascade source.
-    adjacency[src.getOperation()].push_back({dst, /*thisIsCascadeDst=*/false});
-    // dst records src as a peer for which it is the cascade destination.
-    adjacency[dst.getOperation()].push_back({src, /*thisIsCascadeDst=*/true});
+    unsigned idx = adjacency.edges.size();
+    adjacency.edges.push_back({src, dst});
+    if (isa<LogicalTileOp>(src.getOperation()))
+      adjacency.tileToEdges[src.getOperation()].push_back(idx);
+    if (isa<LogicalTileOp>(dst.getOperation()))
+      adjacency.tileToEdges[dst.getOperation()].push_back(idx);
+  }
+  return adjacency;
+}
+
+void SequentialPlacer::attachCascadePeerNotes(
+    InFlightDiagnostic &diag, LogicalTileOp logicalTile,
+    const CascadeAdjacency &adjacency) const {
+  auto it = adjacency.tileToEdges.find(logicalTile.getOperation());
+  if (it == adjacency.tileToEdges.end())
+    return;
+  for (unsigned idx : it->second) {
+    auto [edgeSrc, edgeDst] = adjacency.edges[idx];
+    bool thisIsSrc = edgeSrc.getOperation() == logicalTile.getOperation();
+    TileLike peer = thisIsSrc ? edgeDst : edgeSrc;
+    auto peerPos = resolvePeerPosition(peer, result);
+    if (!peerPos)
+      continue;
+    diag.attachNote(peer.getLoc())
+        << "cascade " << (thisIsSrc ? "destination" : "source")
+        << " peer placed at (" << peerPos->col << ", " << peerPos->row << ")";
   }
 }
 
 bool SequentialPlacer::satisfiesCascadeAdjacency(
     LogicalTileOp logicalTile, TileID candidate,
     const CascadeAdjacency &adjacency) const {
-  auto it = adjacency.find(logicalTile.getOperation());
-  if (it == adjacency.end())
+  auto it = adjacency.tileToEdges.find(logicalTile.getOperation());
+  if (it == adjacency.tileToEdges.end())
     return true;
 
-  for (auto peerEntry : it->second) {
-    LogicalTileOp peer = peerEntry.first;
-    bool thisIsCascadeDst = peerEntry.second;
-    auto placedIt = result.find(peer.getOperation());
-    if (placedIt == result.end())
-      continue; // Peer not placed yet — constraint deferred.
-    TileID peerPos = placedIt->second;
-
-    // Cascade lowering only supports source-north-of-dest (vertical) or
-    // source-west-of-dest (horizontal). So when this tile is the dst, the
-    // peer (=src) must be at (peer.col, peer.row+1) [one row higher = north]
-    // or (peer.col-1, peer.row) [one col left = west] of `candidate`.
-    bool ok;
-    if (thisIsCascadeDst) {
-      // peer must be one row above OR one column left of candidate.
-      ok = (peerPos.col == candidate.col && peerPos.row == candidate.row + 1) ||
-           (peerPos.row == candidate.row && peerPos.col == candidate.col - 1);
-    } else {
-      // peer must be one row below OR one column right of candidate.
-      ok = (peerPos.col == candidate.col && peerPos.row == candidate.row - 1) ||
-           (peerPos.row == candidate.row && peerPos.col == candidate.col + 1);
-    }
-    if (!ok)
+  // Same predicate as AIELowerCascadeFlowsPass: src must be one row North
+  // or one column West of dst (rows increase upward).
+  for (unsigned idx : it->second) {
+    auto [edgeSrc, edgeDst] = adjacency.edges[idx];
+    bool thisIsSrc = edgeSrc.getOperation() == logicalTile.getOperation();
+    TileLike peer = thisIsSrc ? edgeDst : edgeSrc;
+    auto peerPos = resolvePeerPosition(peer, result);
+    if (!peerPos)
+      continue;
+    TileID srcPos = thisIsSrc ? candidate : *peerPos;
+    TileID dstPos = thisIsSrc ? *peerPos : candidate;
+    if (!targetModel->isSouth(srcPos.col, srcPos.row, dstPos.col, dstPos.row) &&
+        !targetModel->isEast(srcPos.col, srcPos.row, dstPos.col, dstPos.row))
       return false;
   }
   return true;

test/place-tiles/sequential_placer/test_place_cascade.mlir

@@ -10,10 +10,7 @@
 
 // RUN: aie-opt --split-input-file --aie-place-tiles %s | FileCheck %s
 
-// Cascade chain with no constraints — sequential greedy fill places head at
-// (0,2); each subsequent tile in the chain must be south or east of its peer.
-// South of (0,2) is row 1 which is not a CoreTile, so the chain extends east
-// along row 2.
+// Unconstrained chain extends east along row 2 (south of row 2 is non-core).
 // CHECK-LABEL: @cascade_chain_unconstrained
 module @cascade_chain_unconstrained {
   aie.device(npu1) {
@@ -38,8 +35,7 @@ module @cascade_chain_unconstrained {
 
 // -----
 
-// Cascade chain with the head pinned to the top of column 0 — chain extends
-// south down the same column.
+// Pinned head at top of column 0 — chain extends south down the column.
 // CHECK-LABEL: @cascade_chain_vertical
 module @cascade_chain_vertical {
   aie.device(npu1) {
@@ -64,8 +60,7 @@ module @cascade_chain_vertical {
 
 // -----
 
-// Cascade between two pre-pinned tiles that satisfy the constraint — placer
-// is a pass-through, no errors.
+// Pre-pinned compatible cascade — placer is a pass-through.
 // CHECK-LABEL: @cascade_pinned_compatible
 module @cascade_pinned_compatible {
   aie.device(npu1) {
@@ -79,3 +74,59 @@ module @cascade_pinned_compatible {
     // CHECK-NOT: aie.logical_tile
   }
 }
+
+// -----
+
+// Pinned peer appears in IR after unconstrained peer; %a's candidates are
+// filtered against %b's pin coords (not just placed peers), so %a → (0,4).
+// CHECK-LABEL: @cascade_pinned_anchor_after_unconstrained
+module @cascade_pinned_anchor_after_unconstrained {
+  aie.device(npu1) {
+    // CHECK-DAG: %[[A:.*]] = aie.tile(0, 4)
+    %a = aie.logical_tile<CoreTile>(?, ?)
+    // CHECK-DAG: %[[B:.*]] = aie.tile(0, 3)
+    %b = aie.logical_tile<CoreTile>(0, 3)
+    aie.cascade_flow(%a, %b)
+    aie.core(%a) { aie.end }
+    aie.core(%b) { aie.end }
+    // CHECK-NOT: aie.logical_tile
+  }
+}
+
+// -----
+
+// Hybrid cascade: TileOp src + LogicalTileOp dst. Dst adapts to TileOp's
+// coords via the TileLike interface (would otherwise be silently dropped).
+// CHECK-LABEL: @cascade_hybrid_tileop_peer
+module @cascade_hybrid_tileop_peer {
+  aie.device(npu1) {
+    %src = aie.tile(1, 3)
+    // CHECK-DAG: %[[D:.*]] = aie.tile(1, 2)
+    %dst = aie.logical_tile<CoreTile>(?, ?)
+    aie.cascade_flow(%src, %dst)
+    aie.core(%src) { aie.end }
+    aie.core(%dst) { aie.end }
+    // CHECK-NOT: aie.logical_tile
+  }
+}
+
+// -----
+
+// Mid-chain pin: A→B→C with B pinned at (1,3). Both ends adapt: A west, C south.
+// CHECK-LABEL: @cascade_chain_pinned_middle
+module @cascade_chain_pinned_middle {
+  aie.device(npu1) {
+    // CHECK-DAG: %[[A:.*]] = aie.tile(0, 3)
+    %a = aie.logical_tile<CoreTile>(?, ?)
+    // CHECK-DAG: %[[B:.*]] = aie.tile(1, 3)
+    %b = aie.logical_tile<CoreTile>(1, 3)
+    // CHECK-DAG: %[[C:.*]] = aie.tile(1, 2)
+    %c = aie.logical_tile<CoreTile>(?, ?)
+    aie.cascade_flow(%a, %b)
+    aie.cascade_flow(%b, %c)
+    aie.core(%a) { aie.end }
+    aie.core(%b) { aie.end }
+    aie.core(%c) { aie.end }
+    // CHECK-NOT: aie.logical_tile
+  }
+}

test/place-tiles/sequential_placer/test_place_errors.mlir

@@ -182,16 +182,15 @@ module @compute_tiles_exhausted {
 
 // -----
 
-// Cascade chain too long for a single column on npu1_1col (4 core rows).
-// Head pinned at row 5, chain extends south to (0,4), (0,3), (0,2), then the
-// fifth tile has no valid neighbour (row 1 is non-core and there's no col 1).
+// 5-tile chain on npu1_1col (4 core rows): chain exhausts the column.
 module @cascade_chain_unsatisfiable {
   aie.device(npu1_1col) {
     %a = aie.logical_tile<CoreTile>(0, 5)
     %b = aie.logical_tile<CoreTile>(?, ?)
     %c = aie.logical_tile<CoreTile>(?, ?)
     %d = aie.logical_tile<CoreTile>(?, ?)
     // CHECK: error: no available compute tiles for placement (cascade adjacency unsatisfiable)
+    // CHECK: note: cascade source peer placed at (0, 2)
     %e = aie.logical_tile<CoreTile>(?, ?)
     aie.cascade_flow(%a, %b)
     aie.cascade_flow(%b, %c)
@@ -207,14 +206,13 @@ module @cascade_chain_unsatisfiable {
 
 // -----
 
-// Cascade direction that the cascade_flow verifier accepts (cardinal
-// adjacent) but the lowering rejects: source must be south or west of dest,
-// not north or east. Here %src at (0,4) is north of %dst at (0,5), so the
-// placer rejects when %src is placed after %dst.
+// Verifier-legal but lowering-illegal direction: lowering requires src to be
+// one row North or one column West of dst (rows increase upward).
 module @cascade_pinned_wrong_direction {
   aie.device(npu1) {
+    // CHECK: error: tile (0, 5) violates cascade adjacency
+    // CHECK: note: cascade source peer placed at (0, 4)
     %dst = aie.logical_tile<CoreTile>(0, 5)
-    // CHECK: error: tile (0, 4) violates cascade adjacency with an already-placed peer
     %src = aie.logical_tile<CoreTile>(0, 4)
     aie.cascade_flow(%src, %dst)
     aie.core(%src) { aie.end }
@@ -224,6 +222,37 @@ module @cascade_pinned_wrong_direction {
 
 // -----
 
+// Partial constraint (col=0) is incompatible with pinned src at (1,3).
+// Exercises candidate-filter rejection (col-0 tiles exist, none satisfy).
+module @cascade_partial_constraint_unsatisfiable {
+  aie.device(npu1) {
+    %src = aie.logical_tile<CoreTile>(1, 3)
+    // CHECK: error: no compute tile available matching constraint (0, ?) and cascade adjacency
+    // CHECK: note: cascade source peer placed at (1, 3)
+    %dst = aie.logical_tile<CoreTile>(0, ?)
+    aie.cascade_flow(%src, %dst)
+    aie.core(%src) { aie.end }
+    aie.core(%dst) { aie.end }
+  }
+}
+
+// -----
+
+// Hybrid cascade with conflicting partial constraint on the logical peer.
+module @cascade_hybrid_unsatisfiable {
+  aie.device(npu1) {
+    %src = aie.tile(1, 3)
+    // CHECK: error: no compute tile available matching constraint (3, ?) and cascade adjacency
+    // CHECK: note: cascade source peer placed at (1, 3)
+    %dst = aie.logical_tile<CoreTile>(3, ?)
+    aie.cascade_flow(%src, %dst)
+    aie.core(%src) { aie.end }
+    aie.core(%dst) { aie.end }
+  }
+}
+
+// -----
+
 // Mixed input/output exceeds capacity
 module @mixed_channels_exceed_capacity {
   aie.device(npu1) {