batch mem2reg to process all variables in a single pass#547
Open
LegNeato wants to merge 3 commits intoRust-GPU:mainfrom
Open
batch mem2reg to process all variables in a single pass#547LegNeato wants to merge 3 commits intoRust-GPU:mainfrom
LegNeato wants to merge 3 commits intoRust-GPU:mainfrom
Conversation
Collaborator
Author
|
@eddyb can you review this one? |
mem2reg processed each variable independently. This O(V*N) complexity caused multi-minute hangs on large post-inline functions. Batch all operations into single passes over the instructions. Fixes Rust-GPU#546. Measured on the proof-of-space-gpu shader: mem2reg drops from 228s to 18s (13x), total link from 236s to 26s. Also adds unit tests.
Firestar99
reviewed
Mar 18, 2026
Member
Firestar99
left a comment
Firestar99
left a comment
There was a problem hiding this comment.
Looks sane to me, then again, I never looked at our mem2reg pass before. What's really interesting to me are the access patters of this pass, and how it reads and writes instructions, surely useful info for implementing some more efficient data structures for storing a module.
The SPIR-V spec allows OpLine/OpNoLine between OpPhi instructions at the start of a block. Account for this in the phi search so it doesn't stop early at a debug instruction. Adds two tests for the phi search boundary behavior.
Add doc comment to construct_access_chain_info explaining that access chain indices must be scalar integers per the SPIR-V spec, and that the constants map only tracks u32 (matching what rustc emits). Add test for a u64 constant index that is valid SPIR-V but not resolved by mem2reg, verifying the variable is not promoted.
Contributor
|
As big improvement as it is, 18 seconds for |
Collaborator
Author
|
Yeah, I didn't want to do any major changes and just moved multiple loops to batches. I can take a closer look after we get to a steady state. |
Collaborator
Author
|
@Firestar99 @eddyb any stamp here? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mem2reg processed each variable independently.
This O(V*N) complexity caused multi-minute hangs on large post-inline functions.
Batch all operations into single passes over the instructions.
Fixes #546. Measured on the proof-of-space-gpu shader: mem2reg drops from 228s to 18s, total link from 236s to 26s.
Also adds unit tests.
Disclosure: largely AI.