Skip to content

Drop failure case for stablehlo.dynamic_broadcast_in_dim #2770

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Drop failure case for stablehlo.dynamic_broadcast_in_dim #2770

wants to merge 4 commits into from

Conversation

rsuderman
Copy link

The failure to broadcast dynamically makes the assumption the input dynamic shape could be expanded by being 1. This should be handled by an earlier trasform to materialize a known broadcast if we intend to support both cases.

@rsuderman
Drop failure case for stablehlo.dynamic_broadcast_in_dim
e0b92bb 
The failure to broadcast dynamically makes the assumption the input
dynamic shape could be expanded by being `1`. This should be handled by
an earlier trasform to materialize a known broadcast if we intend to
support both cases.
@rsuderman rsuderman force-pushed the broadcast_dyn branch 2 times, most recently from 6a197ca to e0b92bb Compare April 7, 2025 20:34
@rsuderman
Copy link
Author

@GleasonK if you do not mind looking over. I believe there is a way to safely introduce the conditional broadcasting but it depends on how stablehlo.get_dimension_size is lowered.

@jpienaar jpienaar requested a review from GleasonK April 10, 2025 17:35
@jpienaar
Copy link
Member

Could you say more about requirement on lowering of get_dimension_size? Also is that what's happening at the moment via the lowering to linalg path?

@rsuderman
Copy link
Author

Could you say more about requirement on lowering of get_dimension_size? Also is that what's happening at the moment via the lowering to linalg path?

There is no current lowering in the stablehlo repo. iree side we just lower to tensor.dim and tensor.from_elements instead of concatting everything together.

Something worth noting is to have the conditional broadcast (e.g. ? being a broadcasting 1) we could materialize out an additional 1 dim then broadcast across it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GleasonK GleasonK Awaiting requested review from GleasonK

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rsuderman @jpienaar