[CtrlPkt] Generate connection operations in the overlay
#1063
Conversation
Yu-Zhewen
requested review from
MaheshRavishankar,
nirvedhmeshram,
yzhang93,
Abhishek-Varma and
jtuyls
as code owners
| @@ -32,11 +32,6 @@ void AMDAIEDialect::initializeAMDAIETypes() { | |||
| LogicalResult LogicalObjectFifoType::verify( | |||
| llvm::function_ref<mlir::InFlightDiagnostic()> emitError, | |||
| mlir::MemRefType elementType, unsigned depth) { | |||
| if (llvm::any_of(elementType.getShape(), [](auto dimSize) { | |||
There was a problem hiding this comment.
The constraint has been removed because the control packet may require a dynamic shape, depending on the reconfiguration content. Additionally, if a device is reconfigured multiple times, the size of the control packet may vary with each reconfiguration.
reference: Xilinx/mlir-aie#1776
| passManager.addPass(createCSEPass()); | ||
| passManager.addPass(createCanonicalizerPass()); |
There was a problem hiding this comment.
Is there a specific reason for this removal?
There was a problem hiding this comment.
They were initially introduced in #1012 only to tidy up the generated overlay. However, with this PR, both the generated connection and logicalobjectfifo.placeholder operations (which are marked as pure) are required for the later DMA generation. Also, I don’t think the CSE and Canonicalize here affect things like control code size anyway.
There was a problem hiding this comment.
However, with this PR, both the generated connection and logicalobjectfifo.placeholder operations (which are marked as pure) are required for the later DMA generation.
Right, but the placeholders shouldn't be removed because they are used by the connections? Just making sure, if you keep the cse pass, it would still work, or would it break?
There was a problem hiding this comment.
Yeah, in this case, the placeholders are kept but the connections are still eliminated since they don't have a user yet.
There was a problem hiding this comment.
Without canonicalize and cse, the generated code looks like this
%channel = amdaie.channel(%tile_0_0, 0, port_type = DMA, direction = MM2S)
%channel_0 = amdaie.channel(%tile_0_0, 0, port_type = CTRL, direction = S2MM)
%0 = amdaie.logicalobjectfifo.placeholder{%tile_0_0} : !amdaie.logicalobjectfifo<memref<?xi32>>
%1 = amdaie.logicalobjectfifo.placeholder{%tile_0_0} : !amdaie.logicalobjectfifo<memref<?xi32>>
%2 = amdaie.connection(%1 {%channel_0}, %0 {%channel}) {connection_type = #amdaie<connection_type Packet>} : (!amdaie.logicalobjectfifo<memref<?xi32>>, !amdaie.logicalobjectfifo<memref<?xi32>>)
%channel_1 = amdaie.channel(%tile_0_0, 1, port_type = DMA, direction = MM2S)
%channel_2 = amdaie.channel(%tile_0_1, 0, port_type = CTRL, direction = S2MM)
%3 = amdaie.logicalobjectfifo.placeholder{%tile_0_0} : !amdaie.logicalobjectfifo<memref<?xi32>>
%4 = amdaie.logicalobjectfifo.placeholder{%tile_0_1} : !amdaie.logicalobjectfifo<memref<?xi32>>
%5 = amdaie.connection(%4 {%channel_2}, %3 {%channel_1}) {connection_type = #amdaie<connection_type Packet>} : (!amdaie.logicalobjectfifo<memref<?xi32>>, !amdaie.logicalobjectfifo<memref<?xi32>>)
.......
There was a problem hiding this comment.
I see, we will have to fix that at some point to make it stable, but that needs some further thought with the connection ops being pure, so not needed in this PR for me.
… ops from Dead-Code Elimination (#1111) Following the discussion in #1063, `connection` ops generated for the control overlay do not initially have DMA users, as these are only added later when the content of control packets is determined. However, since `connection` ops are marked as pure in the IR, they can be wrongly eliminated by CSE and Canonicalization if they have no users at that stage.
Instead of generating
flowoperations, we now directly insertconnectionoperations into the overlay. The motivation behind this change is that theseconnectionoperations will be useful for generating control packet DMAs in subsequent steps.