Skip to content

[pmo] Eliminate dead flat namespace tuple numbering from PMOMemoryUseCollector. #21645

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jan 4, 2019

TLDR: This does not eliminate the struct/tuple flat namespace from Predictable
Mem Opts. Just the tuple specific flat namespace code from PMOMemoryUseCollector
that we were computing and then throwing away. I explain below in more detail.

First note that this is cruft from when def-init and pmo were one pass. What we
were doing here was maintaing a flattened tuple namespace while we were
collecting uses in PMOMemoryUseCollector. We never actually used them for
anything since we recomputed this information including information about
structs in PMO itself! So this information was truly completely dead.

This commit removes that and related logic and from a maintenance standpoint
makes PMOMemoryUseCollector a simple visitor that doesn't have any real special
logic in it beyond the tuple scalarization.

NOTE: I split this commit into two parts. 7175e17 only removes the flat namespace tuple numbering to ease review, while 7b7ccdc removes additional code that becomes dead. I did this to ease review.

…Collector.

TLDR: This does not eliminate the struct/tuple flat namespace from Predictable
Mem Opts. Just the tuple specific flat namespace code from PMOMemoryUseCollector
that we were computing and then throwing away. I explain below in more detail.

First note that this is cruft from when def-init and pmo were one pass. What we
were doing here was maintaing a flattened tuple namespace while we were
collecting uses in PMOMemoryUseCollector. We never actually used them for
anything since we recomputed this information including information about
structs in PMO itself! So this information was truly completely dead.

This commit removes that and related logic and from a maintenance standpoint
makes PMOMemoryUseCollector a simple visitor that doesn't have any real special
logic in it beyond the tuple scalarization.
@gottesmm gottesmm requested review from slavapestov and atrick January 4, 2019 21:23
@gottesmm
Copy link
Contributor Author

gottesmm commented Jan 4, 2019

@swift-ci smoke test

//
// This will cause the dataflow to stop propagating any information at the
// use block.
addElementUses(User, PMOUseKind::Escape);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the change requires a bit of explanation. Previously what was happening is that PredictableMemOpts was completely ignoring the field, type here. So we basically had PredictableMemOpts processing the same use multiple times. The effect is the same as just having one use: stopping the propagation of available values.

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome.

@gottesmm gottesmm merged commit 5da00da into swiftlang:master Jan 4, 2019
@gottesmm gottesmm deleted the pr-925cb5305b32ae67b973b608cd43dfb08b84ccd7 branch January 4, 2019 22:15
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.

2 participants