Skip to content

[Common] Update CombineSourceLayoutTransform pass to fold broadcast#23511

Merged
Abhishek-Varma merged 5 commits intoiree-org:mainfrom
Abhishek-Varma:map_gather_broadcast
Mar 11, 2026
Merged

[Common] Update CombineSourceLayoutTransform pass to fold broadcast#23511
Abhishek-Varma merged 5 commits intoiree-org:mainfrom
Abhishek-Varma:map_gather_broadcast

Conversation

@Abhishek-Varma
Copy link
Copy Markdown
Contributor

@Abhishek-Varma Abhishek-Varma commented Feb 18, 2026

This commit makes following updates :-

  1. Folds named/generic broadcast op into MapLoadOp.
  2. Inserts identity MapLoadOp only for "complex" relayout => chains length ≥ 2, has reshape (expand/collapse), and has non-extract_slice op (transpose, pad, copy, broadcast).
  3. Avoids map_load for simple chains (e.g. single extract_slice) to prevent large memref.alloca and stack size issues.
  4. Adds pass flag/option test-combine-non-complex-chains for lit-testing each op in isolation.

@Abhishek-Varma
Copy link
Copy Markdown
Contributor Author

Ping for review.

@hanhanW
Copy link
Copy Markdown
Contributor

hanhanW commented Feb 25, 2026

Ping for review.

I'll take a look later today. I need to catch up context from the issue.

Copy link
Copy Markdown
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.

High-overview question:

This is done to prevent creating MapLoadOp for simple primitives
like tensor.extract_slice since it unneccesarily ends up creating an
empty tensor for MapLoad's destination -> which in turn leads to
creation of big memref.alloca ops later in the pipeline causing stack
size limit issue.

Do we really need the folding for extract_slice op? The implementation is looking for chained relayout op, should we explicitly match the case to reflect what you're saying here?

I approved the previous PR that allows extract_slice op because it is a mirror of the destionatino folding, but I wonder if we need it for source folding.

Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Copy link
Copy Markdown
Contributor

@Max191 Max191 left a comment

Choose a reason for hiding this comment

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

A couple of quick comments

Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Copy link
Copy Markdown
Contributor

@Max191 Max191 left a comment

Choose a reason for hiding this comment

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

Nice! It looks good now (% some nits) aside from the testing. I think we just need a pass option to test non-complex chains and then testing will be much easier.

Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.h Outdated
@Max191
Copy link
Copy Markdown
Contributor

Max191 commented Mar 5, 2026

Also, please update the description to be consistent with the new changes. It is still referring to the old implementation.

@Abhishek-Varma Abhishek-Varma requested a review from Max191 March 6, 2026 09:47
Copy link
Copy Markdown
Contributor

@Max191 Max191 left a comment

Choose a reason for hiding this comment

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

Nice work, it LGTM now. Just make sure to address the comments about the tests before landing.

@Max191
Copy link
Copy Markdown
Contributor

Max191 commented Mar 6, 2026

Also, let's wait for @hanhanW to give one more review before landing.

Copy link
Copy Markdown
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.

Just one question for op collection and one nit for implementation.

Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Comment thread compiler/src/iree/compiler/Codegen/Common/CombineLayoutTransformation.cpp Outdated
Copy link
Copy Markdown
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.

Thanks!

Co-authored-by: Cursor <cursoragent@cursor.com>

Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
@Abhishek-Varma Abhishek-Varma enabled auto-merge (squash) March 11, 2026 06:01
@Abhishek-Varma Abhishek-Varma merged commit af99947 into iree-org:main Mar 11, 2026
107 of 111 checks passed
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