Skip to content

[5.6,SILOptimizer] Let visitTransitiveEndBorrows take SILValues. #41008

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

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Jan 26, 2022

5.6 Summary: Fix compiler crash during dead code elimination when handling patterns involving borrows of unchecked_ownership_conversions.

Explanation: When dealing with nested borrow scopes, dead code elimination must not shorten outer borrow scopes to end prior to the end of inner borrow scopes. To ensure that, the ends of outer borrow scopes are marked live, using the visitTransitiveEndBorrows which sees through reborrows.

Previously, that API took a BorrowedValue. BorrowedValue, however, has not been taught about one particular, exotic form of borrow scopes, namely those begun with unchecked_ownership_conversion instructions. The result was that a BorrowedValue couldn't be constructed from such instructions, and the value passed to visitTransitiveEndBorrows contained a null BorrowedValue.

Here, rather than teaching BorrowedValue to deal with unchecked_ownership_conversion (the correct long-term solution), for now, loosen the restrictions on visitTransitiveEndBorrows so that it takes a SILValue rather than a BorrowedValue.

Issue: rdar://87985420
Original PR: #41007
Testing: New regression tests, Swift CI
Reviewed by: Andrew Trick
Risk: Low.
Scope: Bug fix

Previously, visitTransitiveEndBorrows took BorrowedValues.  However,
there is at least one kind of borrow--namely,
unchecked_ownership_conversion insts--that is not currently permitted by
the BorrowedValue API.  The long term fix is to make BorrowedValue
handle such instructions.  For now, change visitTransitiveEndBorrows to
take SILValues so that unchecked_ownership_conversion can be passed to
the API.

rdar://87985420
@nate-chandler nate-chandler requested a review from atrick January 26, 2022 00:02
@nate-chandler nate-chandler requested a review from a team as a code owner January 26, 2022 00:02
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

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.

Approved for CCC

@nate-chandler
Copy link
Contributor Author

@swift-ci please nominate

@tbkka tbkka changed the title [SILOptimizer] Let visitTransitiveEndBorrows take SILValues. [5.6,SILOptimizer] Let visitTransitiveEndBorrows take SILValues. Jan 26, 2022
@nate-chandler
Copy link
Contributor Author

@swift-ci please test windows platform

@nate-chandler nate-chandler merged commit 73746e0 into swiftlang:release/5.6 Jan 26, 2022
@nate-chandler nate-chandler deleted the cherry-pick/release/5.6/rdar87985420 branch January 26, 2022 04:49
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.6 labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants