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

Adding CloneToConsumersPass and ElideAsyncTransfersPass. #20103

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

benvanik
Copy link
Collaborator

@benvanik benvanik commented Feb 26, 2025

CloneToConsumersPass performs affinity analysis and clones any clonable producer that is used by multiple affinities per-user. It's effectively the inverse of a CSE step and is intended to undo CSE that created the affinity ambiguity.

ElideAsyncTransfersPass does a global analysis to try to find transfers that are redundant and elides them. Unfortunately resource usage refinement is somewhat broken when transfers are turned into clones so the pass is currently disabled. The pass on its own is fine and shows a large reduction in transfers when applied.

The primary case where this arises today is CSE collapsing a splat or splat-like dispatch that is consumed by transfers or dispatches on different affinities.

@benvanik benvanik requested a review from rsuderman February 26, 2025 16:07
@benvanik benvanik force-pushed the users/benvanik/clone-no-yuck branch 4 times, most recently from 57a757d to 54eeac4 Compare February 27, 2025 21:27
benvanik added 6 commits March 3, 2025 08:48
These transfers can arise during stream conversion if there was
ambiguity in the source IR.
This performs affinity analysis and clones any clonable producer that is
used by multiple affinities per-user. It's effectively the inverse of a
CSE step and is intended to undo CSE that created the affinity ambiguity.

The primary case where this arises today is CSE collapsing a splat or
splat-like dispatch that is consumed by transfers or dispatches on
different affinities.
There's larger issues to fix in resource usage analysis and lifetime
assignment.
@benvanik benvanik force-pushed the users/benvanik/clone-no-yuck branch from 54eeac4 to aef6e37 Compare March 3, 2025 18:29
@benvanik
Copy link
Collaborator Author

benvanik commented Mar 3, 2025

@rsuderman after banging my head against the desk for a week I need to switch to other stuff - this should be an improvement for you by having splats/splat-like dispatches cloned to consumer affinities but may still have unneeded copies (no more than before, just unfortunately no fewer). I'll loop back and see if I can remove some of the transfers in a bit.

@benvanik benvanik marked this pull request as ready for review March 3, 2025 18:58
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.

1 participant