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

[pmo] Eliminate incomplete support for promoting enums. #17543

Merged

Conversation

gottesmm
Copy link
Member

This was never implemented correctly way back in 2013-2014. It was originally
added I believe so we could DI checks, but the promotion part was never added.

Given that DI is now completely split from PMO, we can just turn this off and if
necessary add it back on master "properly".

rdar://41161408

This was never implemented correctly way back in 2013-2014. It was originally
added I believe so we could DI checks, but the promotion part was never added.

Given that DI is now completely split from PMO, we can just turn this off and if
necessary add it back on master "properly".

rdar://41161408
@gottesmm
Copy link
Member Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Member Author

I am fine with either reviewers.

@swift-ci swift-ci merged commit 70e757a into apple:master Jun 27, 2018
@gottesmm gottesmm deleted the pr-81a6b3cbe30cc676e69e9b5a9f2095fbeaaf8826 branch June 27, 2018 03:19
@atrick
Copy link
Member

atrick commented Jun 27, 2018

@gottesmm your change looks safe. I'm not quite sure where it was broken and will have to trust you that it was never kicking in downstream somewhere. If you wanted to be sure you could just compare the stripped SIL from the benchmarks.

@gottesmm
Copy link
Member Author

@atrick let me elaborate a bit. The problem is that code for handling this wasn't added in a bunch of places in Predictable Mem Opts. A few examples:

@atrick
Copy link
Member

atrick commented Jun 27, 2018

@gottesmm Yeah, the code you removed is pretty obviously not supposed to run in this pass!

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.

None yet

3 participants