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.

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

Zero padding on MemTiles #1874

Merged
merged 82 commits into from
Nov 18, 2024
Merged

Zero padding on MemTiles #1874

merged 82 commits into from
Nov 18, 2024

Conversation

pvasireddy-amd
Copy link
Collaborator

@pvasireddy-amd pvasireddy-amd commented Oct 23, 2024

This PR introduces the ability to apply zero padding on MemTiles in 3 dimensions. Specifically, it allows padding to be specified at the object FIFO level for MemTile objects through the addition of a padDimensions attribute.

In python code:

of_out = object_fifo(
    "out",
    MemTile,
    ShimTile,
    1,
    memRef_ty2,
    dimensionsToStream=[(5, 5), (5, 5)],
    padDimensions=[(2, 0), (3, 0)],
) 

Lowers to:

aie.objectfifo @out(%tile_0_1 dimensionsToStream [<size = 5, stride = 5>, <size = 5, stride = 5>], {%tile_0_0}, 1 : i32) 
{padDimensions = #aie<bd_pad_layout_array[<const_pad_before = 2, const_pad_after = 0>, <const_pad_before = 3, const_pad_after = 0>]>} : !aie.objectfifo<memref<56xi32>>

Padding can also be applied in the runtime sequence using writebds or through dma_bd_chain.

@pvasireddy-amd pvasireddy-amd changed the title Zero padding on MemTiles [WIP] Zero padding on MemTiles Oct 23, 2024
Copy link
Contributor

github-actions bot commented Nov 13, 2024

Coverage Report

Created: 2024-11-18 17:50

Click here for information about interpreting this report.

FilenameFunction CoverageLine CoverageRegion CoverageBranch Coverage
AIE/IR/AIEDialect.cpp 91.11% 82.63% 85.16% 75.23%
AIE/Transforms/AIEObjectFifoStatefulTransform.cpp 100.00% 93.32% 90.41% 84.87%
AIEX/IR/AIEXDialect.cpp 100.00% 86.21% 89.26% 79.70%
AIEX/Transforms/AIECtrlPacketToDma.cpp 100.00% 96.64% 80.00% 68.18%
AIEX/Transforms/AIEDMATasksToNPU.cpp 86.36% 83.53% 87.91% 83.33%
AIEX/Transforms/AIEDmaToNpu.cpp 96.15% 89.18% 76.80% 60.48%
Totals 93.92% 87.68% 86.62% 78.14%
Generated by llvm-cov -- llvm version 14.0.0

@pvasireddy-amd
Copy link
Collaborator Author

To DO: Cleanup and moving checks to verifier in AIEDMATasksToNpu

@pvasireddy-amd pvasireddy-amd marked this pull request as ready for review November 14, 2024 17:30
@pvasireddy-amd pvasireddy-amd changed the title [WIP] Zero padding on MemTiles Zero padding on MemTiles Nov 14, 2024
@AndraBisca
Copy link
Collaborator

AndraBisca commented Nov 15, 2024

My latest PR that was merged into main has some updates to the documentation and objectfifo python API. Could you please add some similar style documentation for the mem tile padding in the programming guide as well? I think somewhere at the end of Section 2c should be good. This can be a different PR btw, after this one is merged.

Copy link
Collaborator

@AndraBisca AndraBisca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just these comments to address and it should be good to merge.

@@ -1317,7 +1329,6 @@ struct AIEObjectFifoStatefulTransformPass
auto consumerWireType = WireBundle::DMA;
std::set<TileOp>
objectFifoTiles; // track cores to check for loops during unrolling

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo?

lib/Dialect/AIEX/Transforms/AIEDMATasksToNPU.cpp Outdated Show resolved Hide resolved
lib/Dialect/AIEX/Transforms/AIEDMATasksToNPU.cpp Outdated Show resolved Hide resolved
lib/Dialect/AIEX/Transforms/AIEDMATasksToNPU.cpp Outdated Show resolved Hide resolved
lib/Dialect/AIEX/Transforms/AIEDMATasksToNPU.cpp Outdated Show resolved Hide resolved
@@ -133,7 +133,7 @@ module @ndDMAObjFifoAIE2 {
// this case between two adjacent tiles, we need to use DMAs if a data
// layout transformation with dimensionsToStream and dimensionsFromStream was specified.
aie.objectfifo @of0 (%tile12 dimensionsToStream [<size = 16, stride = 1>, <size = 16, stride = 16>, <size = 1, stride = 1>], // transpose
{%tile13 dimensionsFromStream [<size = 1, stride = 1>]},
{%tile13 dimensionsFromStream [<size = 1, stride = 1>]},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

@pvasireddy-amd pvasireddy-amd added this pull request to the merge queue Nov 18, 2024
Merged via the queue into main with commit 7e01df8 Nov 18, 2024
52 checks passed
@pvasireddy-amd pvasireddy-amd deleted the zero_pad branch November 18, 2024 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants