Skip to content

Commit ed7d103

Browse files
committed
[CIR] Revert aie.device to not having any result
Returning a handle was too disruptive to the MLIR AIE unit tests. Controlling a specific aie.device by the runtime can be done later without using the result as a handler.
1 parent 3f114b0 commit ed7d103

File tree

2 files changed

+37
-38
lines changed

2 files changed

+37
-38
lines changed

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

+17-17
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ class AIE_Op<string mnemonic, list<Trait> traits = []> :
2929
def AIE_DeviceOp: AIE_Op<"device", [
3030
AIETarget,
3131
SymbolTable, SingleBlock, NoTerminator, IsolatedFromAbove
32-
]>, Results<(outs Index:$result)> {
32+
]> {
3333
let summary = "Define an AIE design targetting a complete device";
3434
let description = [{
3535
This operation describes a design that executes on a particular AIEngine device.
36-
It does not replace the
37-
default toplevel module in MLIR, the intention is that this could be the case
38-
in the future.
36+
37+
It does not replace the default toplevel module in MLIR since it can be
38+
possible to have several devices in the same module.
3939

4040
When using this operation, all resources in a physical device are available and
4141
the design does not need to be concerned with other potential users of a physical
@@ -48,7 +48,7 @@ def AIE_DeviceOp: AIE_Op<"device", [
4848

4949
Example:
5050
```
51-
%device = aie.device(xcvc1902) {
51+
aie.device(xcvc1902) {
5252
%tile = aie.tile(1, 1)
5353
%CORE = aie.core(%tile) { ... }
5454
}
@@ -643,13 +643,13 @@ def AIE_PacketFlowOp: AIE_Op<"packet_flow", [SingleBlockImplicitTerminator<"EndO
643643
let description = [{
644644
A logical packet-switched flow between tiles. During place and
645645
route, this is replaced by MasterSets and PacketRules inside
646-
switchboxes.
647-
648-
The optional attribute keep_pkt_header indicates whether each
649-
data packet's packet header gets preserved at the flow's
646+
switchboxes.
647+
648+
The optional attribute keep_pkt_header indicates whether each
649+
data packet's packet header gets preserved at the flow's
650650
destination. The optional attribute priority_route indicates
651-
whether the packet flow is routed in priority over other flows,
652-
so that they always get allocated with the same master, slave
651+
whether the packet flow is routed in priority over other flows,
652+
so that they always get allocated with the same master, slave
653653
ports, arbiters and master selects (msel).
654654

655655
Example:
@@ -869,9 +869,9 @@ def AIE_DMABDOp: AIE_Op<"dma_bd", []> {
869869
## DMA constant padding on AIE-ML Devices
870870

871871
AIE-ML devices can apply constant padding at the buffer descriptor level, described with pairs of padding
872-
counts before and after a dimension, to all dimensions in the data layout transformations. The padding
873-
counts can be supplied to the `dma_bd` through an optional argument, an array of "tuple-like" attributes
874-
`bd_pad_layout<const_pad_before, const_pad_after>`, followed by an optional argument `const_val` (default
872+
counts before and after a dimension, to all dimensions in the data layout transformations. The padding
873+
counts can be supplied to the `dma_bd` through an optional argument, an array of "tuple-like" attributes
874+
`bd_pad_layout<const_pad_before, const_pad_after>`, followed by an optional argument `const_val` (default
875875
is 0). All counts are expressed in multiples of the element width.
876876
}];
877877

@@ -1669,7 +1669,7 @@ def AIE_ObjectFifoCreateOp: AIE_Op<"objectfifo", [HasParent<"DeviceOp">, Symbol]
16691669
BDDimLayoutArrayArrayAttr:$dimensionsFromStreamPerConsumer,
16701670
DefaultValuedAttr<BoolAttr, "false">:$via_DMA,
16711671
DefaultValuedAttr<BoolAttr, "false">:$plio,
1672-
// disable_synchronization==true will skip lock generation for
1672+
// disable_synchronization==true will skip lock generation for
16731673
// objectfifo synchronous accesses
16741674
DefaultValuedAttr<BoolAttr, "false">:$disable_synchronization,
16751675
// via_shared_mem==0 means use producer tile's memory module
@@ -2006,11 +2006,11 @@ def AIE_ObjectFifoRegisterProcessOp: AIE_Op<"objectfifo.register_process", []> {
20062006
def AIE_BDChainOp: AIE_Op<"bd_chain", [Symbol, SkipAccessibilityCheckTrait]> {
20072007
let summary = "Definition of a Parametrizable Chain of Buffer Descriptors";
20082008
let description = [{
2009-
This operation allows you to define buffer descriptor chains with parametrizable inputs.
2009+
This operation allows you to define buffer descriptor chains with parametrizable inputs.
20102010
This is useful for common patterns such as double buffering (ping-pong) that may look identical but use different input/output buffers and locks.
20112011
Currently, only buffers and locks are parametrizable.
20122012

2013-
Once defined, an abstract BD chain can be used elsewhere using AIEX ops in the runtime sequence.
2013+
Once defined, an abstract BD chain can be used elsewhere using AIEX ops in the runtime sequence.
20142014
In the future, abstract BD chains will also be usable elsewhere, inside the static configuration.
20152015
At its usage sites, the abstract BD chain will be concretized with the given input arguments.
20162016
}];

lib/CIR/CIRToAIEPasses.cpp

+20-21
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ class CIRToAIETypesAnalysis {
132132
std::any data;
133133
// The AIE operation which is generated
134134
std::optional<mlir::Operation *> newAIEOperation;
135-
// The new operation producing the result instead for replacement, typically
136-
// an UnrealizedConversionCastOp fed by the newAIEOperation
135+
// The new operation producing the result (if any) instead for replacement,
136+
// typically an UnrealizedConversionCastOp fed by the newAIEOperation
137137
std::optional<mlir::Operation *> newProducer;
138138

139139
// Display the content of AIELikeTypesDeconstruction
@@ -246,26 +246,26 @@ class CIRToAIETypesAnalysis {
246246
return detail.value();
247247
}
248248

249+
// Associate to a given aie++ C++ type the lowered AIE operation operation
250+
void setProducerOp(mlir::Type t, mlir::Operation *op, mlir::OpBuilder &b) {
251+
auto &detail = getTypeDetail(t);
252+
detail.newAIEOperation = op;
253+
isAIELoweredType.insert(t);
254+
}
255+
249256
// Associate to a given aie++ C++ type the operation producing the value for
250257
// this type
251258
void setProducerOpWithUCCast(mlir::Type t, mlir::Operation *op,
252259
mlir::OpBuilder &b) {
260+
setProducerOp(t, op, b);
253261
auto &detail = getTypeDetail(t);
254262
detail.newAIEOperation = op;
255263
detail.newProducer = b.create<mlir::UnrealizedConversionCastOp>(
256264
op->getLoc(), t, mlir::ValueRange{op->getResult(0)});
257-
isAIELoweredType.insert(t);
258265
}
259266

260-
// Associate to a given aie++ C++ type the operation producing the value for
261-
// this type
262-
mlir::Operation *getProducerOp(mlir::Type t) {
263-
auto &detail = getTypeDetail(t);
264-
assert(detail.newProducer &&
265-
R"(This type should have an operation registered
266-
with a previous setProducerOp())");
267-
return detail.newProducer.value();
268-
}
267+
// Get the optional operation producing the value for the given aie++ C++ type
268+
auto &getProducerOp(mlir::Type t) { return getTypeDetail(t).newProducer; }
269269

270270
// Get the set of aie++ C++ types which have been lowered to an AIE operation
271271
// producing a value related to that type
@@ -283,9 +283,7 @@ class CIRToAIETypesAnalysis {
283283
for (auto &operand : op->getOpOperands()) {
284284
auto type = operand.get().getType();
285285
if (this->isAIELowered(type)) {
286-
LLVM_DEBUG(op->emitRemark("visitAIEOperands")
287-
<< type << " to "
288-
<< this->getProducerOp(type)->getResult(0));
286+
LLVM_DEBUG(op->emitRemark("visitAIEOperands") << type);
289287
callBack(operand);
290288
}
291289
}
@@ -661,7 +659,7 @@ struct CIRToAIE : CIRToAIEBase<CIRToAIE> {
661659
"using the aie::device");
662660
// Connect directly the aie::tile user to the one produced by the
663661
// matching aie.tile
664-
cast.replaceAllUsesWith(cat->getProducerOp(cast.getType(0)));
662+
cast.replaceAllUsesWith(cat->getProducerOp(cast.getType(0)).value());
665663
oldCastsFromDevice.push_back(cast);
666664
}
667665
});
@@ -728,9 +726,9 @@ struct CIRToAIE : CIRToAIEBase<CIRToAIE> {
728726
// Compute the remapping to be done while cloning from the old
729727
// operands to the new one produced by the lowered AIE operations
730728
cat->visitAIEOperands(scopeOp, [&](auto &operand) {
731-
irm.map(
732-
operand.get(),
733-
cat->getProducerOp(operand.get().getType())->getResult(0));
729+
// Do not remap
730+
if (auto producer = cat->getProducerOp(operand.get().getType()))
731+
irm.map(operand.get(), producer.value()->getResult(0));
734732
});
735733
b.setInsertionPointToStart(&coreOp.getRegion().front());
736734
auto *clone = b.clone(*scopeOp.getOperation(), irm);
@@ -830,8 +828,9 @@ struct CIRToAIE : CIRToAIEBase<CIRToAIE> {
830828
auto deviceOp = b.create<xilinx::AIE::DeviceOp>(u.getLoc(), *deviceId);
831829
// The aie.device requires one block
832830
deviceOp.getRegion().emplaceBlock();
833-
cat->setProducerOpWithUCCast(u.getType(0), deviceOp, b);
834-
u->replaceAllUsesWith(cat->getProducerOp(u.getType(0)));
831+
// Keep for now the UnrealizedConversionCastOp for the aie.device since
832+
// aie.device do not returns value
833+
cat->setProducerOp(u.getType(0), deviceOp, b);
835834
// Note: aie.device does not require a terminator
836835
LLVM_DEBUG(deviceOp.emitRemark("DeviceLowering: end"));
837836
return true;

0 commit comments

Comments
 (0)