Change aie.device op from NoTerminator to SingleBlockImplicitTerminator

include/aie/Dialect/AIE/IR/AIEOps.td

@@ -28,7 +28,7 @@ class AIE_Op<string mnemonic, list<Trait> traits = []> :
 
 def AIE_DeviceOp: AIE_Op<"device", [
     AIETarget, HasParent<"mlir::ModuleOp">,
-    SymbolTable, SingleBlock, NoTerminator, IsolatedFromAbove
+    SymbolTable, SingleBlockImplicitTerminator<"EndOp">, IsolatedFromAbove
   ]> {
   let summary = "Define an AIE design targetting a complete device";
   let description = [{
@@ -63,7 +63,6 @@ def AIE_DeviceOp: AIE_Op<"device", [
   let extraClassDeclaration = [{
     const xilinx::AIE::AIETargetModel &getTargetModel();
   }];
-  let hasVerifier = 1;
 }
 
 def AIE_TileOp: AIE_Op<"tile", [

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

@@ -70,7 +70,7 @@ struct AIEPathfinderPass : AIERoutePathfinderFlowsBase<AIEPathfinderPass> {
       : analyzer(std::move(analyzer)) {}
 
   void runOnOperation() override;
-  void runOnFlow(DeviceOp d, mlir::OpBuilder &builder);
+  void runOnFlow(DeviceOp d);
   void runOnPacketFlow(DeviceOp d, mlir::OpBuilder &builder);
 
   typedef std::pair<mlir::Operation *, Port> PhysPort;

lib/Conversion/AIEToConfiguration/AIEToConfiguration.cpp

@@ -432,6 +432,7 @@ xilinx::AIE::convertTransactionBinaryToMLIR(mlir::MLIRContext *ctx,
                                  AIEDevice::npu1};
   auto device = builder.create<DeviceOp>(loc, devices[columns - 1]);
   device.getRegion().emplaceBlock();
+  DeviceOp::ensureTerminator(device.getBodyRegion(), builder, loc);
   builder.setInsertionPointToStart(device.getBody());
 
   // convert the parsed ops to MLIR

lib/Dialect/AIE/IR/AIEDialect.cpp

@@ -1128,8 +1128,6 @@ const AIETargetModel &DeviceOp::getTargetModel() {
   return xilinx::AIE::getTargetModel(getDevice());
 }
 
-LogicalResult DeviceOp::verify() { return success(); }
-
 //===----------------------------------------------------------------------===//
 // TileOp
 //===----------------------------------------------------------------------===//

lib/Dialect/AIE/Transforms/AIEAssignBuffers.cpp

@@ -463,7 +463,7 @@ struct AIEAssignBufferAddressesPass
 
   void runOnOperation() override {
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
     // Make sure all the buffers have a name
     int counter = 0;
     device.walk<WalkOrder::PreOrder>([&](BufferOp buffer) {

lib/Dialect/AIE/Transforms/AIEAssignLockIDs.cpp

@@ -35,7 +35,7 @@ struct AIEAssignLockIDsPass : AIEAssignLockIDsBase<AIEAssignLockIDsPass> {
 
   void runOnOperation() override {
     DeviceOp device = getOperation();
-    OpBuilder rewriter = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder rewriter = OpBuilder::atBlockTerminator(device.getBody());
 
     // All of the lock ops on a tile, separated into ops which have been
     // assigned to a lock, and ops which have not.

lib/Dialect/AIE/Transforms/AIECanonicalizeDevice.cpp

@@ -45,6 +45,8 @@ struct AIECanonicalizeDevicePass
     deviceOp.getRegion().takeBody(moduleOp.getBodyRegion());
     new (&moduleOp->getRegion(0)) Region(moduleOp);
     moduleOp->getRegion(0).emplaceBlock();
+
+    DeviceOp::ensureTerminator(deviceOp.getBodyRegion(), builder, location);
     OpBuilder builder2 = OpBuilder::atBlockBegin(moduleOp.getBody());
     builder2.insert(deviceOp);
   }

lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp

@@ -174,7 +174,7 @@ struct ConvertFlowsToInterconnect : OpConversionPattern<FlowOp> {
 
 namespace xilinx::AIE {
 
-void AIEPathfinderPass::runOnFlow(DeviceOp d, OpBuilder &builder) {
+void AIEPathfinderPass::runOnFlow(DeviceOp d) {
   // Apply rewrite rule to switchboxes to add assignments to every 'connect'
   // operation inside
   ConversionTarget target(getContext());
@@ -928,15 +928,15 @@ void AIEPathfinderPass::runOnOperation() {
   DeviceOp d = getOperation();
   if (failed(analyzer.runAnalysis(d)))
     return signalPassFailure();
-  OpBuilder builder = OpBuilder::atBlockEnd(d.getBody());
+  OpBuilder builder = OpBuilder::atBlockTerminator(d.getBody());
 
   if (clRouteCircuit)
-    runOnFlow(d, builder);
+    runOnFlow(d);
   if (clRoutePacket)
     runOnPacketFlow(d, builder);
 
   // Populate wires between switchboxes and tiles.
-  builder.setInsertionPointToEnd(d.getBody());
+  builder.setInsertionPoint(d.getBody()->getTerminator());
   for (int col = 0; col <= analyzer.getMaxCol(); col++) {
     for (int row = 0; row <= analyzer.getMaxRow(); row++) {
       TileOp tile;

lib/Dialect/AIE/Transforms/AIEFindFlows.cpp

@@ -226,7 +226,7 @@ class ConnectivityAnalysis {
 static void findFlowsFrom(TileOp op, ConnectivityAnalysis &analysis,
                           OpBuilder &rewriter) {
   Operation *Op = op.getOperation();
-  rewriter.setInsertionPointToEnd(Op->getBlock());
+  rewriter.setInsertionPoint(Op->getBlock()->getTerminator());
 
   std::vector bundles = {WireBundle::Core, WireBundle::DMA};
   for (WireBundle bundle : bundles) {
@@ -276,7 +276,7 @@ struct AIEFindFlowsPass : public AIEFindFlowsBase<AIEFindFlowsPass> {
     ConnectivityAnalysis analysis(d);
     d.getTargetModel().validate();
 
-    OpBuilder builder = OpBuilder::atBlockEnd(d.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(d.getBody());
     for (auto tile : d.getOps<TileOp>()) {
       findFlowsFrom(tile, analysis, builder);
     }

lib/Dialect/AIE/Transforms/AIEGenerateColumnControlOverlay.cpp

@@ -187,7 +187,7 @@ struct AIEGenerateColumnControlOverlayPass
   void runOnOperation() override {
     DeviceOp device = getOperation();
     const auto &targetModel = device.getTargetModel();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     if (targetModel.getTargetArch() == AIEArch::AIE1)
       return; // Disable this pass for AIE1; AIE1 support NYI.
@@ -314,7 +314,7 @@ struct AIEGenerateColumnControlOverlayPass
     auto availableShimChans =
         getAvailableShimChans(device, shimTile, shimWireBundle, isShimMM2S);
 
-    builder.setInsertionPointToEnd(device.getBody());
+    builder.setInsertionPoint(device.getBody()->getTerminator());
     for (auto tOp : ctrlTiles) {
       if (tOp->hasAttr("controller_id"))
         ctrlPktFlowID =

lib/Dialect/AIE/Transforms/AIELowerCascadeFlows.cpp

@@ -30,7 +30,7 @@ struct AIELowerCascadeFlowsPass
   void runOnOperation() override {
     DeviceOp device = getOperation();
     const auto &targetModel = device.getTargetModel();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     std::set<TileOp> tilesWithCascadeFlow;
     DenseMap<TileOp, WireBundle> cascadeInputsPerTile;

lib/Dialect/AIE/Transforms/AIEObjectFifoRegisterProcess.cpp

@@ -99,13 +99,13 @@ struct AIEObjectFifoRegisterProcessPass
 
   void runOnOperation() override {
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
     DenseMap<ObjectFifoCreateOp, std::queue<Value>> consumersPerFifo;
     //===----------------------------------------------------------------------===//
     // Generate access patterns
     //===----------------------------------------------------------------------===//
     for (auto registerOp : device.getOps<ObjectFifoRegisterProcessOp>()) {
-      builder.setInsertionPointToEnd(device.getBody());
+      builder.setInsertionPoint(device.getBody()->getTerminator());
       ObjectFifoCreateOp objFifo = registerOp.getObjectFifo();
       auto elementType =
           llvm::dyn_cast<AIEObjectFifoType>(objFifo.getElemType())

lib/Dialect/AIE/Transforms/AIEObjectFifoStatefulTransform.cpp

@@ -655,10 +655,8 @@ struct AIEObjectFifoStatefulTransformPass
     // if none exists, create one
     TileOp objFifoTileOp = target.getProducerTileOp();
     if (producerMem == nullptr) {
-      if (device->getNumRegions() != 1)
-        assert(false && "expected num regions for device op");
       OpBuilder::InsertionGuard g(builder);
-      builder.setInsertionPointToEnd(device.getBody());
+      builder.setInsertionPoint(device.getBody()->getTerminator());
       auto newMemOp =
           builder.create<MemOp>(builder.getUnknownLoc(), objFifoTileOp);
       {
@@ -727,10 +725,8 @@ struct AIEObjectFifoStatefulTransformPass
     // if none exists, create one
     TileOp objFifoTileOp = op.getProducerTileOp();
     if (producerDMA == nullptr) {
-      if (device->getNumRegions() != 1)
-        assert(false && "expected num regions for device op");
       OpBuilder::InsertionGuard g(builder);
-      builder.setInsertionPointToEnd(device.getBody());
+      builder.setInsertionPoint(device.getBody()->getTerminator());
       auto newDMAOp = builder.create<ShimDMAOp>(
           builder.getUnknownLoc(), builder.getIndexType(), objFifoTileOp);
       {
@@ -881,10 +877,8 @@ struct AIEObjectFifoStatefulTransformPass
     // if none exists, create one
     TileOp objFifoTileOp = target.getProducerTileOp();
     if (producerDMA == nullptr) {
-      if (device->getNumRegions() != 1)
-        assert(false && "expected num regions for device op");
       OpBuilder::InsertionGuard g(builder);
-      builder.setInsertionPointToEnd(device.getBody());
+      builder.setInsertionPoint(device.getBody()->getTerminator());
       auto newDMAOp =
           builder.create<MemTileDMAOp>(builder.getUnknownLoc(), objFifoTileOp);
       {
@@ -1384,7 +1378,7 @@ struct AIEObjectFifoStatefulTransformPass
     DeviceOp device = getOperation();
     LockAnalysis lockAnalysis(device);
     DMAChannelAnalysis dmaAnalysis(device);
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
     auto ctx = device->getContext();
     auto producerWireType = WireBundle::DMA;
     auto consumerWireType = WireBundle::DMA;
@@ -1546,7 +1540,7 @@ struct AIEObjectFifoStatefulTransformPass
                 producerChan.channel, 0, producer.getDimensionsToStreamAttr(),
                 producer.getPadDimensionsAttr());
       // generate objectFifo allocation info
-      builder.setInsertionPoint(&device.getBody()->back());
+      builder.setInsertionPoint(device.getBody()->getTerminator());
 
       if (producer.getProducerTileOp().isShimTile())
         createObjectFifoAllocationInfo(
@@ -1568,7 +1562,7 @@ struct AIEObjectFifoStatefulTransformPass
         createDMA(device, builder, consumer, consumerChan.direction,
                   consumerChan.channel, 1, consumerDims, nullptr);
         // generate objectFifo allocation info
-        builder.setInsertionPoint(&device.getBody()->back());
+        builder.setInsertionPoint(device.getBody()->getTerminator());
 
         // If we have PLIO then figure out the direction and make that a PLIO
         if (producer.getPlio()) {
@@ -1826,7 +1820,7 @@ struct AIEObjectFifoStatefulTransformPass
     }
     // make global symbols to replace the to be erased ObjectFifoCreateOps
     for (auto createOp : device.getOps<ObjectFifoCreateOp>()) {
-      builder.setInsertionPointToStart(&device.getBodyRegion().front());
+      builder.setInsertionPointToStart(device.getBody());
       auto sym_name = createOp.getName();
       createOp->setAttr(SymbolTable::getSymbolAttrName(),
                         builder.getStringAttr("__erase_" + sym_name));

lib/Dialect/AIEX/Transforms/AIECreateBroadcastPacket.cpp

@@ -50,7 +50,7 @@ struct AIEBroadcastPacketPass
   void runOnOperation() override {
 
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     for (auto broadcastpacket : device.getOps<BroadcastPacketOp>()) {
       Region &r = broadcastpacket.getPorts();

lib/Dialect/AIEX/Transforms/AIECreateCores.cpp

@@ -69,7 +69,7 @@ struct AIECreateCoresPass : public AIECreateCoresBase<AIECreateCoresPass> {
   void runOnOperation() override {
 
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     DenseMap<TileID, Operation *> tiles;
     DenseMap<Operation *, CoreOp> cores;

lib/Dialect/AIEX/Transforms/AIECreateLocks.cpp

@@ -135,7 +135,7 @@ struct AIECreateLocksPass : public AIECreateLocksBase<AIECreateLocksPass> {
   void runOnOperation() override {
 
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     TokenAnalysis TA(device);
     TA.runAnalysis();

lib/Dialect/AIEX/Transforms/AIELowerMemcpy.cpp

@@ -101,7 +101,7 @@ struct AIELowerMemcpyPass : public AIELowerMemcpyBase<AIELowerMemcpyPass> {
   void runOnOperation() override {
 
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     // Setup FlowOps
     // Since memcpy moves data from one memory module to another, we use

lib/Dialect/AIEX/Transforms/AIELowerMulticast.cpp

@@ -49,7 +49,7 @@ struct AIELowerMulticastPass : public AIEMulticastBase<AIELowerMulticastPass> {
   void runOnOperation() override {
 
     DeviceOp device = getOperation();
-    OpBuilder builder = OpBuilder::atBlockEnd(device.getBody());
+    OpBuilder builder = OpBuilder::atBlockTerminator(device.getBody());
 
     for (auto multicast : device.getOps<MulticastOp>()) {
       Region &r = multicast.getPorts();

lib/Dialect/AIEX/Transforms/AIESubstituteShimDMAAllocations.cpp

@@ -25,18 +25,15 @@ using namespace mlir;
 using namespace xilinx;
 using namespace xilinx::AIEX;
 
-struct DMAConfigureTaskForOpPattern : RewritePattern {
+namespace {
 
-  DMAConfigureTaskForOpPattern(MLIRContext *ctx)
-      : RewritePattern(DMAConfigureTaskForOp::getOperationName(),
-                       PatternBenefit(1), ctx) {}
+struct DMAConfigureTaskForOpPattern
+    : public mlir::OpRewritePattern<DMAConfigureTaskForOp> {
 
-  LogicalResult matchAndRewrite(Operation *op_any,
+  using OpRewritePattern::OpRewritePattern;
+
+  LogicalResult matchAndRewrite(DMAConfigureTaskForOp op,
                                 PatternRewriter &rewriter) const override {
-    DMAConfigureTaskForOp op = llvm::dyn_cast<DMAConfigureTaskForOp>(op_any);
-    if (!op) {
-      return failure();
-    }
     AIE::DeviceOp device = op->getParentOfType<AIE::DeviceOp>();
 
     AIE::ShimDMAAllocationOp alloc_op =
@@ -67,20 +64,15 @@ struct AIESubstituteShimDMAAllocationsPass
 
     // Convert DMAConfigureTaskForOps that reference shim DMA allocations
     // to regular DMAConfigureTaskOps
-    ConversionTarget target(getContext());
-    target.addLegalDialect<AIEXDialect>();
-    target.addIllegalOp<DMAConfigureTaskForOp>();
     RewritePatternSet patterns(&getContext());
     patterns.insert<DMAConfigureTaskForOpPattern>(&getContext());
 
-    GreedyRewriteConfig rewriter_config = GreedyRewriteConfig();
-    if (failed(applyPatternsAndFoldGreedily(device, std::move(patterns),
-                                            rewriter_config))) {
-      signalPassFailure();
-    }
+    (void)applyPatternsAndFoldGreedily(device, std::move(patterns));
   }
 };
 
+} // namespace
+
 std::unique_ptr<OperationPass<AIE::DeviceOp>>
 AIEX::createAIESubstituteShimDMAAllocationsPass() {
   return std::make_unique<AIESubstituteShimDMAAllocationsPass>();

python/dialects/aie.py

@@ -290,7 +290,7 @@ def __init__(
         )
 
     def get_name(self):
-        return self.result.get_name()
+        return self.sym_name.value if self.sym_name else self.result.get_name()
 
     def __str__(self):
         return str(self.result)
@@ -511,7 +511,7 @@ def __init__(
 
 
 core = region_op(Core, terminator=lambda *_: EndOp())
-device = region_op(Device)
+device = region_op(Device, terminator=lambda *_: EndOp())
 switchbox = region_op(
     lambda tile, *, loc=None, ip=None: SwitchboxOp(T.index(), tile, loc=loc, ip=ip)
 )

test/objectFifo-stateful-transform/dynamic_lowering_flag_test.mlir

@@ -62,7 +62,6 @@
 // CHECK:       }
 // CHECK:       aie.end
 // CHECK:     } {dynamic_objfifo_lowering = true}
-// CHECK:     aie.shim_dma_allocation @input_fifo(MM2S, 0, 0)
 // CHECK:     %core_0_4 = aie.core(%tile_0_4) {
 // CHECK:       %c0 = arith.constant 0 : index
 // CHECK:       %c1 = arith.constant 1 : index
@@ -80,8 +79,9 @@
 // CHECK:         aie.use_lock(%input_fifo2_cons_prod_lock, Release, 1)
 // CHECK:         aie.use_lock(%output_fifo2_cons_lock, Release, 1)
 // CHECK:       }        
-// CHECK:        aie.end
-// CHECK:      }
+// CHECK:       aie.end
+// CHECK:     }
+// CHECK:     aie.shim_dma_allocation @input_fifo(MM2S, 0, 0)
 
 module {
   aie.device(npu1_1col) {

test/objectFifo-stateful-transform/dynamic_lowering_test.mlir

@@ -35,7 +35,6 @@
 // CHECK:       %input_fifo_cons_lock = aie.lock(%tile_0_0, 1) {init = 0 : i32, sym_name = "input_fifo_cons_lock"}
 // CHECK:       aie.flow(%tile_0_0, DMA : 0, %tile_0_2, DMA : 0)
 // CHECK:       aie.flow(%tile_0_2, DMA : 0, %tile_0_0, DMA : 0)
-// CHECK:       aie.shim_dma_allocation @input_fifo(MM2S, 0, 0)
 // CHECK:       %buffer_0_2 = aie.buffer(%tile_0_2) : memref<2xindex> 
 // CHECK:       %core_0_2 = aie.core(%tile_0_2) {
 // CHECK:         %c0 = arith.constant 0 : index
@@ -194,6 +193,8 @@
 // CHECK:         memref.store %18, %buffer_0_2[%c0_0] : memref<2xindex>
 // CHECK:         aie.end
 // CHECK:       }
+// CHECK:       aie.shim_dma_allocation @input_fifo(MM2S, 0, 0)
+
 
 module {
   aie.device(npu1_1col) {

test/objectFifo-stateful-transform/link_test_AIE1.mlir

@@ -26,7 +26,6 @@
 // CHECK:           %[[VAL_7:.*]] = aie.lock(%[[VAL_0]], 0) {init = 0 : i32, sym_name = "of1_lock_0"}
 // CHECK:           aie.flow(%[[VAL_0]], DMA : 0, %[[VAL_1]], DMA : 0)
 // CHECK:           %[[VAL_8:.*]] = aie.external_buffer {sym_name = "ext_buff_in"} : memref<16xi32>
-// CHECK:           aie.shim_dma_allocation @of1(MM2S, 0, 2)
 // CHECK:           %[[VAL_9:.*]] = aie.shim_dma(%[[VAL_0]]) {
 // CHECK:             %[[VAL_10:.*]] = aie.dma_start(MM2S, 0, ^bb1, ^bb2)
 // CHECK:           ^bb1:  // 2 preds: ^bb0, ^bb1
@@ -37,6 +36,7 @@
 // CHECK:           ^bb2:  // pred: ^bb0
 // CHECK:             aie.end
 // CHECK:           }
+// CHECK:           aie.shim_dma_allocation @of1(MM2S, 0, 2)
 // CHECK:           %[[VAL_11:.*]] = aie.mem(%[[VAL_1]]) {
 // CHECK:             %[[VAL_12:.*]] = aie.dma_start(S2MM, 0, ^bb1, ^bb3)
 // CHECK:           ^bb1:  // 2 preds: ^bb0, ^bb2