Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CreateLogicalObjectFifoLink] Fail if accesses not contiguous #696

Merged
merged 3 commits into from
Aug 29, 2024
Merged

[CreateLogicalObjectFifoLink] Fail if accesses not contiguous #696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
676f451
squash em
newling Aug 29, 2024
485c633
Merge branch 'main' into link_creation_additional_check
newling Aug 29, 2024
5edde2f
Merge branch 'main' into link_creation_additional_check
newling Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions ...iler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIECreateLogicalObjectFifoLink.cpp
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions ...r/plugins/target/AMD-AIE/iree-amd-aie/Transforms/test/create_logical_objectfifo_link.mlir
View file
Open in desktop
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
Loading