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

[Dispatch] Add pattern to bubble expand through extract 1/2 #19325

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

IanWood1
Copy link
Contributor

@IanWood1 IanWood1 commented Nov 27, 2024

This is the 1/2 changes needed to reland #18857 (with an open PR #19113).

Adds pattern to bubble up expand shape through extract slice. i.e expand(extract) to extract(expand). This only supports the case where the expanded dimensions are not modified by the extract slice and there are no dynamic dimensions.

This is important because tensor.expand_shape ops cannot be cloned while tensor.extract_slice ops can be cloned. So, if the expand_shape gets stuck on the bottom of the extract_slice it will block it from being cloned and the extract_slice will have to be put into its own dispatch.

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.

LGTM, just one nit about the doc.

// TODO: move this upstream with other tensor bubbling patterns.
/// Bubbles a `tensor.expand_shape` op through a `tensor.extract_slice` op. This
/// pattern only gets applied when the `extract_slice` doesn't modify dimensions
/// that are expanded by the `expand_shape`.
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, it only works for static shapes? Dynamic shapes could be very tricky, and I think we're not going to support that in the near future. So we want to document that it only works for static shapes.

Copy link
Contributor Author

@IanWood1 IanWood1 Dec 2, 2024

Choose a reason for hiding this comment

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

Yeah, only static shapes so I updated the comment to reflect that

@IanWood1 IanWood1 merged commit 529cd89 into iree-org:main Dec 3, 2024
38 checks passed
@IanWood1 IanWood1 deleted the bubble_expand_through_extract branch December 3, 2024 16:36
giacs-epic pushed a commit to giacs-epic/iree that referenced this pull request Dec 4, 2024
…#19325)

This is the 1/2 changes needed to reland
iree-org#18857 (with an open PR
iree-org#19113).

Adds pattern to bubble up expand shape through extract slice. i.e
`expand(extract)` to `extract(expand)`. This only supports the case
where the expanded dimensions are not modified by the extract slice and
there are no dynamic dimensions.

This is important because `tensor.expand_shape` ops _cannot be cloned_
while `tensor.extract_slice` ops _can be cloned_. So, if the
`expand_shape` gets stuck on the bottom of the `extract_slice` it will
block it from being cloned and the `extract_slice` will have to be put
into its own dispatch.

---------

Signed-off-by: Ian Wood <[email protected]>
Signed-off-by: Giacomo Serafini <[email protected]>
IanWood1 added a commit that referenced this pull request Jan 8, 2025
Reland after fixing sdxl int8 regressions via
#19012.

Running CI revealed further performance regressions that have pending
patches: #19325 and
#19326.

This reverts commit 8d3faf8.

---------

Signed-off-by: Ian Wood <[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.

2 participants