Skip to content

[ome] Just erase the destructure rather than trying to get rid of dea… #27606

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

Merged

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Oct 10, 2019

…d instructions due to simplifyInstruction.

This ensures that we do not run into any iterator invalidation issues. It does
worsen the passes output slightly, but avoiding the complexity is worth it and
other later passes will clean up the dead code.

rdar://55811732
(cherry picked from commit 1b29d86)


— CCC Info —

Radar: rdar://problem/55811732
Explanation: The optimizer was being too aggressive around using a recursive API that deleted instructions when they only had uses by debug value instructions. Turns out in certain cases we were not careful enough around iterator invalidation and we delete an instruction causing the invalidation. I changed the code in question to just remove the specific instruction in question and allow for any dead uses we found to be cleaned up by later passes.
Scope of issue: Caused the compiler to crash in an edge case.
Origination: This was introduced as part of the ownership sil work to lower destructure operations -> non-ownership sil variants.
Risk: Low.
Testing: Added a regression test for this case and verified that we do not crash any more.
Reviewers: Erik Eckstein

…d instructions due to simplifyInstruction.

This ensures that we do not run into any iterator invalidation issues. It does
worsen the passes output slightly, but avoiding the complexity is worth it and
other later passes will clean up the dead code.

rdar://55811732
(cherry picked from commit 1b29d86)
@gottesmm gottesmm requested a review from eeckstein October 10, 2019 20:10
@gottesmm gottesmm requested a review from a team as a code owner October 10, 2019 20:10
@gottesmm
Copy link
Contributor Author

@swift-ci test

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm!

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 2dbf54d

@gottesmm
Copy link
Contributor Author

@swift-ci test os x platform

@gottesmm
Copy link
Contributor Author

@swift-ci nominate

@tkremenek tkremenek merged commit 6d433ae into swiftlang:swift-5.1-branch Oct 11, 2019
@gottesmm gottesmm deleted the swift-5.1-branch-rdar55811732 branch October 11, 2019 17:57
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.

4 participants