Skip to content

Commit

Permalink
[CreateLogicalObjectFifoLink] Fail if accesses not contiguous (#696)
Browse files Browse the repository at this point in the history
I was looking into this pass because convolution lowering hits 

```
error: 'amdaie.circular_dma_cpy_nd' op access pattern of strided operation overlaps with next one, which is not supported for now
```

But looking at the code now, it looks like it'll currently only work for
the the access patterns are perfectly contiguous? If that's the case, I
can look into fixing it (for both cases -- overlapping and
non-contiguous).

I left a comment in the code describing my thinking
  • Loading branch information
newling authored Aug 29, 2024
1 parent bf99fb8 commit 1f32c6c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ namespace mlir::iree_compiler::AMDAIE {
/// these two access patterns overlap by both accessing elements in range [32,
/// 63].
template <CopyOpOperateOn OperateOn>
LogicalResult checkForNoOverlappingAccessPatterns(
const SmallVector<std::pair<DoublyStridedCopyOpInterface, int64_t>>
&stridedOps) {
LogicalResult checkForContiguousAccessPatterns(
ArrayRef<std::pair<DoublyStridedCopyOpInterface, int64_t>> stridedOps) {

for (auto &&[i, stridedOpAndOffset] : llvm::enumerate(stridedOps)) {
DoublyStridedCopyOpInterface stridedOp = stridedOpAndOffset.first;
std::optional<int64_t> extent;
Expand All @@ -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();
Expand Down Expand Up @@ -81,9 +93,8 @@ LogicalResult createLogicalObjectFifoLink(
for (Operation *userOp : logicalObjectFifo->getUsers()) {
if (auto stridedOp = dyn_cast<DoublyStridedCopyOpInterface>(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<AMDAIE::LogicalObjectFifoFromMemrefOp>(
Expand Down Expand Up @@ -130,11 +141,11 @@ LogicalResult createLogicalObjectFifoLink(
// Check that access patterns are not overlapping between consumers
// respectively producers.
if (failed(
checkForNoOverlappingAccessPatterns<CopyOpOperateOn::Target>(ins))) {
checkForContiguousAccessPatterns<CopyOpOperateOn::Target>(ins))) {
return failure();
}
if (failed(
checkForNoOverlappingAccessPatterns<CopyOpOperateOn::Source>(outs))) {
checkForContiguousAccessPatterns<CopyOpOperateOn::Source>(outs))) {
return failure();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<memref<8x8x4x8xi32, 2>>
%3 = amdaie.circular_dma_cpy_nd(%1[0] [1024] [1], %0[] [] []) : (!amdaie.logicalobjectfifo<memref<3x32x64xi32, 1>>, !amdaie.logicalobjectfifo<memref<32x1024xi32>>)
%4 = amdaie.circular_dma_cpy_nd(%1[1, 0] [1, 1024] [2048, 1], %0[] [] []) : (!amdaie.logicalobjectfifo<memref<3x32x64xi32, 1>>, !amdaie.logicalobjectfifo<memref<32x1024xi32>>)
// 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<memref<3x32x64xi32, 1>>, !amdaie.logicalobjectfifo<memref<32x1024xi32>>)
%6 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[] [] []) : (!amdaie.logicalobjectfifo<memref<8x8x4x8xi32, 2>>, !amdaie.logicalobjectfifo<memref<3x32x64xi32, 1>>)
return
Expand Down Expand Up @@ -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<memref<3x32x64xi32, 1>>, !amdaie.logicalobjectfifo<memref<32x1024xi32>>)
%4 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[1, 0] [1, 1024] [2048, 1]) : (!amdaie.logicalobjectfifo<memref<8x8x4x8xi32, 2>>, !amdaie.logicalobjectfifo<memref<3x32x64xi32, 1>>)
%5 = amdaie.circular_dma_cpy_nd(%2[] [] [], %1[1, 0] [1, 1024] [1024, 1]) : (!amdaie.logicalobjectfifo<memref<8x8x4x8xi32, 2>>, !amdaie.logicalobjectfifo<memref<3x32x64xi32, 1>>)
// 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<memref<8x8x4x8xi32, 2>>, !amdaie.logicalobjectfifo<memref<3x32x64xi32, 1>>)
return
}
Expand Down Expand Up @@ -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<memref<32x1024xi32>>
// 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<memref<32x64xi32, 1>>
%2 = amdaie.logicalobjectfifo.from_memref %arg2, {} : memref<8x8x4x8xi32, 2> -> !amdaie.logicalobjectfifo<memref<8x8x4x8xi32, 2>>
Expand Down

0 comments on commit 1f32c6c

Please sign in to comment.