Repository navigation
Create missing state output in MSE for allocas - #1942
Merged
Merged
Conversation
phate
previously approved these changes
Oct 5, 2026
phate
approved these changes
Oct 5, 2026
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
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.
I first tried to disqualify dynamic allocas from being simple in the ModRefSummarizer.
At the most, a single file has 99 dynamic allocas, and 12 files have >= 10 dynamic allocas. 164 files have at least one dynamic alloca.
Not all dynamic allocas were simple to begin with, however, so the actual reduction in simple allocas was at most 15, with only the top 10 files seeing >= 4 simple allocas removed.
At the end of the sroa-raware pipeline, the change lead to 60 more loads compared to master, across 14 files (12 up, 2 down).
It also lead to 108 more stores, but I think those might actually be mostly correct, and were always supposed to be there.
Lastly it lead to 11 more allocas being kept.
Next I tried what this PR does: Creating an UndefValue for the memory state output if it does not exist yet when required by something that is not an alloca. This was a much smaller code change, and I think it makes much more sense, as it avoids adding even more special handling of the set of allocas that is or isn't included in the ModRefSummary of functions. I left a suggestion for an assert in the PR as a comment. The assert it not possible right now since the StateMap class not having access to the PointsToGraph, but the whole MSE class is being refactored, so I would rather just add it as a comment that can be fixed later.
The total effect on the sroa-raware pipeline compared to master: 4 more loads, 105 more stores. The load diffs are in 6 files: (+17, +17, +4, -1, -2, -31). The store diffs are in 8 files, which I again think are mostly supposed to be there. We keep 8 more allocas.