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

[Codegen] Use affine.delinearize_index in workgroup distribution #19839

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

krzysz00
Copy link
Contributor

In the interests of getting rid of needless substractions from affine composition, get rid of one of the last remaining manual calls to floorDiv()

The other ProcInfo generators have been converted to delinearize_index by earlier PRs - this one finishes the job.

This should not impact the behavior of generated programs.

In the interests of getting rid of needless substractions from affine
composition, get rid of one of the last remaining manual calls to
floorDiv()

The other ProcInfo generators have been converted to delinearize_index
by earlier PRs - this one finishes the job.

This should not impact the behavior of generated programs.
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

Same here. I can approve it if we are switching to affine.delinerize_index approach. Just one nit about the comment.

Co-authored-by: Han-Chung Wang <[email protected]>
@krzysz00
Copy link
Contributor Author

krzysz00 commented Feb 3, 2025

@hanhanW Yeah, I'm switching us over to affine.delinearize_index because it doesn't lead to bad composition the way that an affine map with floordiv and mod does

// CHECK: %[[D20:.*]] = flow.dispatch.tensor.load %[[D5]], offsets = {{\[}}%[[ARG0]], %[[ARG1]], %[[ARG2]], %[[ARG3]]], sizes = {{\[}}%[[D9]], %[[D12]], %[[D15]], %[[D18]]], strides = [1, 1, 1, 1] : !flow.dispatch.tensor<readonly:tensor<?x?x?x?xf32>>{%[[D0]], %[[D1]], %[[D2]], %[[D3]]} -> tensor<?x?x?x?xf32>
// CHECK: %[[D21:.*]] = tensor.empty(%[[D9]], %[[D12]], %[[D15]], %[[D18]]) : tensor<?x?x?x?xf32>
// CHECK: %[[D22:.*]] = linalg.generic {indexing_maps = [#map9, #map9, #map9], iterator_types = ["parallel", "parallel", "parallel", "parallel"]} ins(%[[D19]], %[[D20]] : tensor<?x?x?x?xf32>, tensor<?x?x?x?xf32>) outs(%[[D21]] : tensor<?x?x?x?xf32>) attrs = {lowering_config = #config} {
// CHECK-DAG: %[[D7:.*]] = affine.apply #map(){{\[}}%[[D1]]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you dont want to use linearize as well. These all seem like linearizations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to keep the PR minimal but also the fact that a lot of that's coming out of the linalg machinery or otherwise isn't as easy to get to

Or at least that's my memory of why I didn't touch anything.

@krzysz00 krzysz00 merged commit 1ed6350 into iree-org:main Feb 4, 2025
43 checks passed
ita9naiwa pushed a commit to ita9naiwa/iree that referenced this pull request Feb 4, 2025
…e-org#19839)

In the interests of getting rid of needless substractions from affine
composition, get rid of one of the last remaining manual calls to
floorDiv()

The other ProcInfo generators have been converted to delinearize_index
by earlier PRs - this one finishes the job.

This should not impact the behavior of generated programs.

---------

Co-authored-by: Han-Chung Wang <[email protected]>
Signed-off-by: Hyunsung Lee <[email protected]>
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