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

Don't wrap singleton ir.Values with tuples during HLO lowering. #22211

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

hawkinsp
Copy link
Member

@hawkinsp hawkinsp commented Jul 1, 2024

In general a JAX value might correspond to multiple HLO values, which is why the HLO lowering represents each value ad a tuple of zero or more ir.Values. However, the common case is that there is exactly one value, and almost all such lists are singletons.

To reduce the number of singleton list and tuple objects allocated during MLIR lowering, instead represent singleton values as unwrapped ir.Values, and only use a tuple if there is not exactly one ir.Value backing a JAX value.

@hawkinsp hawkinsp added the pull ready Ready for copybara import and testing label Jul 1, 2024
@gnecula
Copy link
Collaborator

gnecula commented Jul 1, 2024

This has been a constant source of mistakes for me: where to add/remove the tuples. Thanks!

@hawkinsp hawkinsp force-pushed the singletons branch 2 times, most recently from 1e363eb to 035f4af Compare July 1, 2024 16:10
In general a JAX value might correspond to multiple HLO values, which is why the HLO lowering represents each value as a tuple of zero or more ir.Values. However, the common case is that there is exactly one value, and almost all such lists are singletons.

To reduce the number of singleton list and tuple objects allocated during MLIR lowering, instead represent singleton values as unwrapped ir.Values, and only use a tuple if there is not exactly one ir.Value backing a JAX value.
@hawkinsp hawkinsp requested a review from gnecula July 1, 2024 20:25
@copybara-service copybara-service bot merged commit dffd72e into google:main Jul 3, 2024
14 of 15 checks passed
hawkinsp added a commit to hawkinsp/jax that referenced this pull request Jul 3, 2024
This is similar to google#22211, but for MLIR types instead of MLIR values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants