C++: Better discrimination for union Contents#12184
Merged
MathiasVP merged 1 commit intoFeb 14, 2023
Merged
Conversation
c11218f
into
github:mathiasvp/replace-ast-with-ir-use-usedataflow
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.
This PR fixes a performance problem on the use-use flow branch.
One of the things the dataflow library computes is the possible set of tails for an access path that starts with a specific
Content. We have modelled a read of (or write to) a union using aContentbranch defined by the union type itself (so that you can write to one union member and read it off another).The problem is that this makes the set of tails for a union
Contentthe a lot larger than the set of tails for a specific Field (since there's just 1Contentvalue per union in the database).Performance-wise the best fix would be to model unions as we model regular struct fields (as this would increase the discrimination factor by creating a
Contentbranch for each field in the union instead of 1Contentbranch for each union in the database). However, this would prevent a write to one union field to flow to another union read of a different field. This pattern occurs quite frequently (especially in C projects where it's not actually undefined behavior).Instead, this PR adds another column to the union
Contentbranch that specifies "the size of the field that's currently active in the union". This means there are more unionContentbranches in the database (so that the set of tail access paths is spread out more across thoseContents), while still ensuring that we get dataflow in examples like:Locally, this gives me a large speedup for the Nelson project.