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

Make aiex.npu.dma_memcpy_nd d0 stride explicit #1586

Merged
merged 11 commits into from
Jul 2, 2024
Merged

Conversation

fifield
Copy link
Collaborator

@fifield fifield commented Jun 28, 2024

The stride of the inner dimension is assumed to be one for npu.dma_memcpy_nd (e.g. here). This PR makes it an explicit operand.

replaces #1584

Copy link
Contributor

Coverage Report

Created: 2024-06-28 20:13

Click here for information about interpreting this report.

FilenameFunction CoverageLine CoverageRegion CoverageBranch Coverage
IR/AIEXDialect.cpp 100.00% 90.42% 89.86% 80.85%
Transforms/AIEDmaToNpu.cpp 100.00% 95.77% 91.46% 82.76%
Totals 100.00% 92.94% 90.43% 81.58%
Generated by llvm-cov -- llvm version 14.0.0

Copy link
Collaborator

@andrej andrej 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 to me. Have you tested using a d0 stride != 1 at all?

// strides[0] == 1 is ok iff the tranfer size is a multiple of
// addressGranularity, which is checked below
if (i == 0 && raw_strides[i] == 1)
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good. Technically something like

sizes=[addressGranularity/2, addressGranularity/2]
strides=[addressGranularity/2, 1]

(and similar examples for higher dimensions, with addressGranularity in bytes)
is equivalent to

sizes=[addressGranularity]
strides=[1]

but would currently error.

I think it's a non-issue as this is clearly a contrived example with no real uses. (Except maybe automatically generated code?)

So this looks good to me with this check.

@fifield fifield added this pull request to the merge queue Jul 2, 2024
Merged via the queue into main with commit cd33847 Jul 2, 2024
51 checks passed
@fifield fifield deleted the npu_memcpy_d0_stride branch July 2, 2024 15:24
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.

None yet

2 participants