Skip to content

Commit

Permalink
Clean up and restructure objectfifo MLIR tests (#1966)
Browse files Browse the repository at this point in the history 
Co-authored-by: AndraBisca <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Pranathi Vasireddy <[email protected]>
  • Loading branch information
@AndraBisca @abisca
@github-actions @pvasireddy-amd
4 people authored Feb 18, 2025
1 parent ab9760e commit 5c503f2
Show file tree
Hide file tree
Showing 120 changed files with 3,952 additions and 1,492 deletions.
4 changes: 2 additions & 2 deletions include/aie/Dialect/AIE/IR/AIEOps.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1857,7 +1857,7 @@ def AIE_ObjectFifoAcquireOp: AIE_Op<"objectfifo.acquire", []> {
let arguments = (
ins ObjectFifoPort:$port,
FlatSymbolRefAttr:$objFifo_name,
ConfinedAttr<AIEI32Attr, [IntMinValue<0>]>:$size
ConfinedAttr<AIEI32Attr, [IntMinValue<1>]>:$size
);

let results = (outs AIE_ObjectFifoSubviewType:$subview);
Expand Down Expand Up @@ -1893,7 +1893,7 @@ def AIE_ObjectFifoReleaseOp: AIE_Op<"objectfifo.release", []> {
let arguments = (
ins ObjectFifoPort:$port,
FlatSymbolRefAttr:$objFifo_name,
ConfinedAttr<AIEI32Attr, [IntMinValue<0>]>:$size
ConfinedAttr<AIEI32Attr, [IntMinValue<1>]>:$size
);

let assemblyFormat = [{
Expand Down
13 changes: 7 additions & 6 deletions lib/Dialect/AIE/IR/AIEDialect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,13 +509,20 @@ LogicalResult ObjectFifoCreateOp::verify() {
if (getConsumerTiles().size() > 1)
return emitError(
"`via_shared_mem` can only be used in 1-to-1 object FIFOs");
if (getVia_DMA())
return emitError("`via_shared_mem` and `via_DMA` cannot occur together");
}

if (getRepeatCount().has_value()) {
if (getProducerTileOp().isShimTile())
return emitError("`repeat_count` unavailable for shim tiles");
}

if (getInitValues().has_value()) {
if (getProducerTileOp().isShimTile())
return emitError("`init_values` unavailable for shim tiles");
}

if (getInitValues().has_value()) {
if ((int)getInitValues().value().size() != size())
return emitError("`init_values` does not initialize all objects");
Expand Down Expand Up @@ -880,9 +887,6 @@ ObjectFifoCreateOp ObjectFifoRegisterExternalBuffersOp::getObjectFifo() {
//===----------------------------------------------------------------------===//

LogicalResult ObjectFifoAcquireOp::verify() {
if (acqNumber() < 1)
return emitOpError("must acquire at least one element");

auto parent = getOperation()->getParentOfType<CoreOp>();
if (parent == nullptr)
return emitOpError("must be called from inside a CoreOp");
Expand Down Expand Up @@ -938,9 +942,6 @@ ObjectFifoCreateOp ObjectFifoAcquireOp::getObjectFifo() {
//===----------------------------------------------------------------------===//

LogicalResult ObjectFifoReleaseOp::verify() {
if (relNumber() < 1)
return emitOpError("must release at least one element");

auto parent = getOperation()->getParentOfType<CoreOp>();
if (parent == nullptr)
return emitOpError("must be called from inside a CoreOp");
Expand Down
104 changes: 66 additions & 38 deletions lib/Dialect/AIE/Transforms/AIEObjectFifoStatefulTransform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ struct AIEObjectFifoStatefulTransformPass

// Checks if via_shared_mem attribute of the objectfifo is set and if so
// tries to apply it. If the desired shared memory module is available to
// both producer and consumer then it will be used, otherwise a warning is
// emitted and the original shared memory module is used instead.
// both producer and consumer then it will be used, otherwise an error is
// emitted.
void checkAndApplyViaSharedMemAttribute(ObjectFifoCreateOp createOp,
int &share_direction) {
if (createOp.getViaSharedMem().has_value()) {
Expand All @@ -303,8 +303,9 @@ struct AIEObjectFifoStatefulTransformPass
if (share_direction == newShareDirection)
share_direction = (share_direction == -1) ? 1 : -1;
else
createOp->emitWarning("Memory module specified by `via_shared_mem` "
"is not available as shared memory module");
createOp->emitOpError(
"no access to shared memory module specified by "
"`via_shared_mem`");
}
}
}
Expand Down Expand Up @@ -342,6 +343,7 @@ struct AIEObjectFifoStatefulTransformPass
std::vector<LockOp> createObjectFifoLocks(OpBuilder &builder,
LockAnalysis &lockAnalysis,
ObjectFifoCreateOp op, int numElem,
int joinDistribFactor,
TileOp creation_tile,
int repeatCount) {
std::vector<LockOp> locks;
Expand Down Expand Up @@ -379,7 +381,8 @@ struct AIEObjectFifoStatefulTransformPass
: 0;
int prodLockID = lockAnalysis.getLockID(creation_tile);
assert(prodLockID >= 0 && "No more locks to allocate!");
int prodLockValue = (numElem - initValues) * repeatCount;
int prodLockValue =
(numElem - initValues) * joinDistribFactor * repeatCount;
auto prodLock = builder.create<LockOp>(
builder.getUnknownLoc(), creation_tile, prodLockID, prodLockValue);
prodLock.getOperation()->setAttr(
Expand All @@ -389,7 +392,7 @@ struct AIEObjectFifoStatefulTransformPass

int consLockID = lockAnalysis.getLockID(creation_tile);
assert(consLockID >= 0 && "No more locks to allocate!");
int consLockValue = initValues * repeatCount;
int consLockValue = initValues * joinDistribFactor * repeatCount;
auto consLock = builder.create<LockOp>(
builder.getUnknownLoc(), creation_tile, consLockID, consLockValue);
consLock.getOperation()->setAttr(
Expand Down Expand Up @@ -494,19 +497,20 @@ struct AIEObjectFifoStatefulTransformPass
of_elem_index++;
}
int repeatCount = 1;
int joinDistribFactor = 1;
if (op.getRepeatCount().has_value())
repeatCount = op.getRepeatCount().value();
if (linked) {
if (linkOp->getRepeatCount().has_value())
repeatCount = linkOp->getRepeatCount().value();
if (linkOp->isDistribute())
numElem *= linkOp->getFifoOuts().size();
joinDistribFactor *= linkOp->getFifoOuts().size();
else if (linkOp->isJoin())
numElem *= linkOp->getFifoIns().size();
joinDistribFactor *= linkOp->getFifoIns().size();
objFifoLinks[*linkOp] = op;
}
std::vector<LockOp> locks = createObjectFifoLocks(
builder, lockAnalysis, op, numElem, creation_tile, repeatCount);
builder, lockAnalysis, op, numElem, joinDistribFactor, creation_tile, repeatCount);
buffersPerFifo[op] = buffers;
locksPerFifo[op] = locks;
}
Expand Down Expand Up @@ -1079,11 +1083,32 @@ struct AIEObjectFifoStatefulTransformPass
// !! NOTE !! objectFifos with same producer / consumer tile
// need two counters (accessed based on the ObjectFifoPort)
std::map<std::pair<ObjectFifoCreateOp, ObjectFifoPort>, int> fifoSizes;
// Also, keep a map of the ConstantOps for the indices per OF
// and a map with the ConstantOps for the sizes per OF.
std::map<std::pair<ObjectFifoCreateOp, ObjectFifoPort>,
arith::ConstantOp>
globalIndices;
std::map<std::pair<ObjectFifoCreateOp, ObjectFifoPort>,
arith::ConstantOp>
constantSizes;

int index = 0;
builder.setInsertionPointToStart(&(coreOp.getBody().front()));
Value initVal = builder.create<arith::ConstantOp>(
builder.getUnknownLoc(), builder.getI32IntegerAttr(0));
coreOp.walk([&](ObjectFifoAcquireOp acqOp) {
ObjectFifoCreateOp op = acqOp.getObjectFifo();
ObjectFifoPort port = acqOp.getPort();
if (fifoSizes.find({op, port}) == fifoSizes.end())
if (fifoSizes.find({op, port}) == fifoSizes.end()) {
fifoSizes[{op, port}] = op.size();
auto indexOp = builder.create<arith::ConstantOp>(
initVal.getLoc(), builder.getIndexAttr(index));
globalIndices[{op, port}] = indexOp;
index++;
auto size = builder.create<arith::ConstantOp>(
indexOp.getLoc(), builder.getI32IntegerAttr(op.size()));
constantSizes[{op, port}] = size;
}
});
builder.setInsertionPoint(coreOp);
auto memrefTy =
Expand All @@ -1095,29 +1120,11 @@ struct AIEObjectFifoStatefulTransformPass
/*initial_value*/ nullptr, /*mem_bank*/ nullptr);

// Initialize all counters in the global buffers to 0.
// Also, keep a map of the ConstantOps for the indices per OF
// and a map with the ConstantOps for the sizes per OF.
std::map<std::pair<ObjectFifoCreateOp, ObjectFifoPort>,
arith::ConstantOp>
globalIndices;
std::map<std::pair<ObjectFifoCreateOp, ObjectFifoPort>,
arith::ConstantOp>
constantSizes;
int index = 0;
builder.setInsertionPointToStart(&(coreOp.getBody().front()));
Value initVal = builder.create<arith::ConstantOp>(
builder.getUnknownLoc(), builder.getI32IntegerAttr(0));
for (auto i : fifoSizes) {
auto indexOp = builder.create<arith::ConstantOp>(
initVal.getLoc(), builder.getIndexAttr(index));
globalIndices[i.first] = indexOp;
index++;
auto size = builder.create<arith::ConstantOp>(
indexOp.getLoc(), builder.getI32IntegerAttr(i.second));
constantSizes[i.first] = size;
for (auto i : constantSizes) {
builder.setInsertionPointAfter(i.second);
builder.create<memref::StoreOp>(
size.getLoc(), initVal, globalNextIndex,
ValueRange(ArrayRef({indexOp.getResult()})));
builder.getUnknownLoc(), initVal, globalNextIndex,
ValueRange(ArrayRef({globalIndices[i.first].getResult()})));
}

// Walk the code:
Expand Down Expand Up @@ -1374,6 +1381,26 @@ struct AIEObjectFifoStatefulTransformPass
builder.getBoolAttr(plio));
}

/// Function used to verify that an objectfifo is present in at most one
/// ObjectFifoLinkOp.
void verifyObjectFifoLinks(DeviceOp &device) {
DenseSet<ObjectFifoCreateOp> objectfifoset;
for (ObjectFifoLinkOp link : device.getOps<ObjectFifoLinkOp>()) {
for (ObjectFifoCreateOp inOf : link.getInputObjectFifos()) {
if (objectfifoset.count(inOf))
inOf.emitOpError("objectfifo cannot be in more than one "
"ObjectFifoLinkOp");
objectfifoset.insert(inOf);
}
for (ObjectFifoCreateOp outOf : link.getOutputObjectFifos()) {
if (objectfifoset.count(outOf))
outOf.emitOpError("objectfifo cannot be in more than one "
"ObjectFifoLinkOp");
objectfifoset.insert(outOf);
}
}
}

void runOnOperation() override {
DeviceOp device = getOperation();
LockAnalysis lockAnalysis(device);
Expand All @@ -1384,6 +1411,9 @@ struct AIEObjectFifoStatefulTransformPass
auto consumerWireType = WireBundle::DMA;
std::set<TileOp>
objectFifoTiles; // track cores to check for loops during unrolling

verifyObjectFifoLinks(device);

//===------------------------------------------------------------------===//
// Split objectFifos into a consumer end and producer end if needed
//===------------------------------------------------------------------===//
Expand All @@ -1403,9 +1433,6 @@ struct AIEObjectFifoStatefulTransformPass
// Only FIFOs using DMA are split into two ends;
// skip in shared memory case
if (int share_direction = 0; !requiresDMAs(createOp, share_direction)) {
if (createOp.getRepeatCount().has_value())
createOp->emitWarning("Repeat unavailable for tiles sharing memory; "
"ignoring `repeat_count`");
continue;
}

Expand Down Expand Up @@ -1504,8 +1531,9 @@ struct AIEObjectFifoStatefulTransformPass
share_direction);
} else {
if (createOp.getViaSharedMem().has_value())
createOp->emitWarning("No access to shared memory module; ignoring "
"`via_shared_mem`");
createOp->emitOpError(
"no access to shared memory module specified by "
"`via_shared_mem`");

if (isa<ArrayAttr>(createOp.getElemNumber()))
createOp.setElemNumberAttr(
Expand Down Expand Up @@ -1569,7 +1597,7 @@ struct AIEObjectFifoStatefulTransformPass
producerWireType = producer.getProducerTileOp().isShimTile()
? WireBundle::PLIO
: WireBundle::DMA;
consumerWireType = !(producer.getProducerTileOp().isShimTile())
consumerWireType = consumer.getProducerTileOp().isShimTile()
? WireBundle::PLIO
: WireBundle::DMA;
} else {
Expand Down
78 changes: 78 additions & 0 deletions test/dialect/AIE/objectfifo_bad.mlir
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//===- objectfifo_bad.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
//
// Copyright (C) 2025, Advanced Micro Devices, Inc.
//
//===----------------------------------------------------------------------===//

// RUN: not aie-opt -split-input-file %s 2>&1 | FileCheck %s

// CHECK: 'aie.objectfifo' op does not have enough depths specified for producer and for each consumer.

aie.device(xcve2302) {
%tile12 = aie.tile(1, 2)
%tile13 = aie.tile(1, 3)
%tile23 = aie.tile(2, 3)

aie.objectfifo @of_0 (%tile12, {%tile13, %tile23}, [2, 2]) : !aie.objectfifo<memref<16xi32>>
}

// -----

// CHECK: custom op 'aie.objectfifo' initial values should initialize all objects

aie.device(xcve2302) {
%tile12 = aie.tile(1, 2)
%tile23 = aie.tile(2, 3)

aie.objectfifo @of0 (%tile12, {%tile23}, 3 : i32) : !aie.objectfifo<memref<4xi32>> = [dense<[0, 1, 2, 3]> : memref<4xi32>]
}

// -----

// CHECK: custom op 'aie.objectfifo' initial value should be an elements attribute

aie.device(xcve2302) {
%tile12 = aie.tile(1, 2)
%tile23 = aie.tile(2, 3)

aie.objectfifo @of0 (%tile12, {%tile23}, 1 : i32) : !aie.objectfifo<memref<4xi32>> = [[0, 1, 2, 3]]
}

// -----

// CHECK: inferred shape of elements literal ({{\[}}2, 3]) does not match type ({{\[}}2, 2])

aie.device(xcve2302) {
%tile12 = aie.tile(1, 2)
%tile23 = aie.tile(2, 3)

aie.objectfifo @of0 (%tile12, {%tile23}, 2 : i32) : !aie.objectfifo<memref<2x2xi32>> = [dense<[[4, 5], [6, 7]]> : memref<2x2xi32>,
dense<[[0, 1, 2], [3, 4, 5]]> : memref<2x2xi32>]
}

// -----

// CHECK: `via_shared_mem` can only be used in 1-to-1 object FIFOs

aie.device(xcve2302) {
%tile20 = aie.tile(2, 0)
%tile13 = aie.tile(1, 3)
%tile23 = aie.tile(2, 3)

aie.objectfifo @of_0 (%tile20, {%tile13, %tile23}, 2 : i32) {via_shared_mem = 1 : i32} : !aie.objectfifo<memref<16xi32>>
}

// -----

// CHECK: `via_shared_mem` and `via_DMA` cannot occur together

aie.device(xcve2302) {
%tile12 = aie.tile(1, 2)
%tile13 = aie.tile(1, 3)

aie.objectfifo @of_0 (%tile12, {%tile13}, 2 : i32) {via_shared_mem = 1 : i32, via_DMA = true} : !aie.objectfifo<memref<16xi32>>
}
Loading

0 comments on commit 5c503f2

Please sign in to comment.