From 58dec2d6afa87dff37308fe63c085b01e768c044 Mon Sep 17 00:00:00 2001 From: James Newling Date: Thu, 22 Aug 2024 16:44:55 -0700 Subject: [PATCH] check for exact contiguity --- .../AMDAIECreateLogicalObjectFifoLink.cpp | 46 ++++++++++++------- .../test/create_logical_objectfifo_link.mlir | 6 +-- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECreateLogicalObjectFifoLink.cpp b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECreateLogicalObjectFifoLink.cpp index f9225e612..f736c4f55 100644 --- a/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECreateLogicalObjectFifoLink.cpp +++ b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECreateLogicalObjectFifoLink.cpp @@ -32,9 +32,9 @@ namespace mlir::iree_compiler::AMDAIE { /// these two access patterns overlap by both accessing elements in range [32, /// 63]. template -LogicalResult checkForNoOverlappingAccessPatterns( - const SmallVector> - &stridedOps) { +LogicalResult checkForContiguousAccessPatterns( + ArrayRef> stridedOps) { + for (auto &&[i, stridedOpAndOffset] : llvm::enumerate(stridedOps)) { DoublyStridedCopyOpInterface stridedOp = stridedOpAndOffset.first; std::optional extent; @@ -45,14 +45,26 @@ LogicalResult checkForNoOverlappingAccessPatterns( } if (!extent) { return stridedOp.emitOpError() - << "non-constant access extent is not supported"; + << "has a non-constant access extent, which is not supported"; } int64_t offset = stridedOpAndOffset.second; - if (i < (stridedOps.size() - 1) && - (offset + extent.value()) > stridedOps[i + 1].second) { - return stridedOp.emitOpError() - << "access pattern of strided operation overlaps with next one, " - "which is not supported for now"; + if (i < (stridedOps.size() - 1)) { + if (offset + extent.value() != stridedOps[i + 1].second) { + // TODO(newling) my understanding from the code is that the link + // operation effectively replaces the cumulative offset of each + // circular_dma_cpy_nd with the differential offset with + // the previous circular_dma_cpy_nd in the 'link' list. + // + // This however is hardcoded to a zero offset (later in the pass where + // discardAllNonZeroOffsets is called, offsets are set to zero). This + // effectively is constraining the link operation to only work with + // contiguous access patterns. + // + // Is this a bug? + return stridedOp.emitOpError() + << "has access pattern of which isn't contiguous with next one " + "-- not currently supported."; + } } } return success(); @@ -81,17 +93,15 @@ LogicalResult createLogicalObjectFifoLink( for (Operation *userOp : logicalObjectFifo->getUsers()) { if (auto stridedOp = dyn_cast(userOp)) { if (lastUserOp && stridedOp->getBlock() != lastUserOp->getBlock()) { - logicalObjectFifo->emitError( - "does have copy-like users not residing in the same block"); - return failure(); + return logicalObjectFifo->emitOpError( + "has copy-like users not residing in the same block"); } auto sourceLogicalObjectFifo = dyn_cast( stridedOp.getSource().getDefiningOp()); if (!sourceLogicalObjectFifo) { - stridedOp->emitError( + return stridedOp->emitOpError( "does not have a `LogicalObjectFifoFromMemrefOp` as source"); - return failure(); } if (!lastUserOp || lastUserOp->isBeforeInBlock(stridedOp)) { lastUserOp = stridedOp; @@ -130,11 +140,11 @@ LogicalResult createLogicalObjectFifoLink( // Check that access patterns are not overlapping between consumers // respectively producers. if (failed( - checkForNoOverlappingAccessPatterns(ins))) { + checkForContiguousAccessPatterns(ins))) { return failure(); } if (failed( - checkForNoOverlappingAccessPatterns(outs))) { + checkForContiguousAccessPatterns(outs))) { return failure(); } @@ -188,6 +198,8 @@ struct AMDAIECreateLogicalObjectFifoLinkPass SmallVector shape; (void)discardAllNonZeroOffsets( rewriter, stridedOp, shape); + } else { + assert(false && "Expected a DoublyStridedCopyOpInterface"); } } for (Value output : linkOp.getOuts()) { @@ -196,6 +208,8 @@ struct AMDAIECreateLogicalObjectFifoLinkPass SmallVector shape; (void)discardAllNonZeroOffsets( rewriter, stridedOp, shape); + } else { + assert(false && "Expected a DoublyStridedCopyOpInterface"); } } }); diff --git a/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/create_logical_objectfifo_link.mlir b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/create_logical_objectfifo_link.mlir index 2f915931c..67577eff1 100644 --- a/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/create_logical_objectfifo_link.mlir +++ b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/create_logical_objectfifo_link.mlir @@ -79,7 +79,7 @@ func.func @link_multiple_inputs_with_overlapping_access(%arg0: memref<32x1024xi3 %2 = amdaie.logicalobjectfifo.from_memref %arg2, {} : memref<8x8x4x8xi32, 2> -> !amdaie.logicalobjectfifo> %3 = amdaie.circular_dma_cpy_nd(%1[0] [1024] [1], %0[] [] []) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) %4 = amdaie.circular_dma_cpy_nd(%1[1, 0] [1, 1024] [2048, 1], %0[] [] []) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) - // expected-error @+1 {{access pattern of strided operation overlaps with next one}} + // expected-error @+1 {{op has access pattern of which isn't contiguous with next one}} %5 = amdaie.circular_dma_cpy_nd(%1[1, 0] [1, 1025] [1024, 1], %0[] [] []) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) %6 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[] [] []) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) return @@ -151,7 +151,7 @@ func.func @link_multiple_outputs_with_overlapping_access(%arg0: memref<32x1024xi %3 = amdaie.circular_dma_cpy_nd(%1[] [] [], %0[] [] []) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) %4 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[1, 0] [1, 1024] [2048, 1]) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) %5 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[1, 0] [1, 1024] [1024, 1]) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) - // expected-error @+1 {{access pattern of strided operation overlaps with next one}} + // expected-error @+1 {{op has access pattern of which isn't contiguous with next one}} %6 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[0, 0] [32, 32] [64, 1]) : (!amdaie.logicalobjectfifo>, !amdaie.logicalobjectfifo>) return } @@ -243,7 +243,7 @@ func.func @ensure_no_removal_of_offsets(%arg0: memref<32x1024xi32>, %arg1: memre func.func @link_different_blocks(%arg0: memref<32x1024xi32>, %arg1: memref<32x64xi32, 1>, %arg2: memref<8x8x4x8xi32, 2>) { %0 = amdaie.logicalobjectfifo.from_memref %arg0, {} :memref<32x1024xi32> -> !amdaie.logicalobjectfifo> - // expected-error @+2 {{does have copy-like users not residing in the same block}} + // expected-error @+2 {{has copy-like users not residing in the same block}} // expected-error @+1 {{couldn't create a link operation}} %1 = amdaie.logicalobjectfifo.from_memref %arg1, {} : memref<32x64xi32, 1> -> !amdaie.logicalobjectfifo> %2 = amdaie.logicalobjectfifo.from_memref %arg2, {} : memref<8x8x4x8xi32, 2> -> !amdaie.logicalobjectfifo>