Skip to content

cast optimizer: don't create a critical edge where it's not allowed #14166

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 26, 2018
Merged

cast optimizer: don't create a critical edge where it's not allowed #14166

merged 1 commit into from
Jan 26, 2018

Conversation

eeckstein
Copy link
Contributor

https://bugs.swift.org/browse/SR-6773
rdar://problem/36602263

@eeckstein eeckstein requested a review from shajrawi January 25, 2018 21:47
@eeckstein
Copy link
Contributor Author

@swift-ci test

shajrawi
shajrawi approved these changes Jan 25, 2018
View reviewed changes
Copy link

@shajrawi shajrawi left a comment

Choose a reason for hiding this comment

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

LGTM!

@eeckstein
Copy link
Contributor Author

Explanation: The cast optimizer produces wrong SIL in some cases
Scope: We found this only with enabling the SIL verifier after each pass. But it could be the case that this bug can really cause a compiler crash or miscompile.
Radar (and possibly SR Issue): https://bugs.swift.org/browse/SR-6773, rdar://problem/36602263
Risk: Low. The fix just adds splitting a critical CFG edge and has no other side effects.
Testing: with a regression test

@eeckstein eeckstein merged commit 5b1852d into swiftlang:swift-4.1-branch Jan 26, 2018
@eeckstein eeckstein deleted the fix-castoptimizer-4.1 branch January 26, 2018 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shajrawi shajrawi shajrawi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eeckstein @shajrawi