Skip to content

Commit

Permalink
Change aie.device op from NoTerminator to SingleBlockImplicitTerminat…
Browse files Browse the repository at this point in the history
…or (#1936)
  • Loading branch information
fifield authored Nov 23, 2024
1 parent f7867a9 commit c21dee7
Show file tree
Hide file tree
Showing 42 changed files with 148 additions and 151 deletions.
3 changes: 1 addition & 2 deletions include/aie/Dialect/AIE/IR/AIEOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [{
Expand Down Expand Up @@ -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", [
Expand Down
2 changes: 1 addition & 1 deletion include/aie/Dialect/AIE/Transforms/AIEPasses.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions lib/Conversion/AIEToConfiguration/AIEToConfiguration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions lib/Dialect/AIE/IR/AIEDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1128,8 +1128,6 @@ const AIETargetModel &DeviceOp::getTargetModel() {
return xilinx::AIE::getTargetModel(getDevice());
}

LogicalResult DeviceOp::verify() { return success(); }

//===----------------------------------------------------------------------===//
// TileOp
//===----------------------------------------------------------------------===//
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIE/Transforms/AIEAssignBuffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIE/Transforms/AIEAssignLockIDs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions lib/Dialect/AIE/Transforms/AIECanonicalizeDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/Dialect/AIE/Transforms/AIECreatePathFindFlows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/AIE/Transforms/AIEFindFlows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIE/Transforms/AIELowerCascadeFlows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/AIE/Transforms/AIEObjectFifoRegisterProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
20 changes: 7 additions & 13 deletions lib/Dialect/AIE/Transforms/AIEObjectFifoStatefulTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
{
Expand Down Expand Up @@ -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);
{
Expand Down Expand Up @@ -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);
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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()) {
Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIEX/Transforms/AIECreateBroadcastPacket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIEX/Transforms/AIECreateCores.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIEX/Transforms/AIECreateLocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIEX/Transforms/AIELowerMemcpy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/AIEX/Transforms/AIELowerMulticast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 9 additions & 17 deletions lib/Dialect/AIEX/Transforms/AIESubstituteShimDMAAllocations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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>();
Expand Down
4 changes: 2 additions & 2 deletions python/dialects/aie.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion test/objectFifo-stateful-transform/link_test_AIE1.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit c21dee7

Please sign in to comment.