5.9: [MoveChecker] Complete lifetimes before checking. #68381
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nate-chandler
force-pushed
the
cherrypick/release/5.9/gh68328
branch
from
bc2413f to
4cda39e
Compare
nate-chandler
force-pushed
the
cherrypick/release/5.9/gh68328
branch
from
4cda39e to
ea5af4f
Compare
|
@swift-ci please test |
end_borrow was not considered as a lifetime ending use, leading to insertion of redundant end_borrows at boundaries.
A dead end block can be visited multiple times on each of its predecessors path. Use BasicBlockWorklist::pushIfNotVisited api for insertion.
…gOperand kinds Not all BorrowingOperand introduce a BorrowedValue. Add a new api to get the defining value for use in InteriorLivenessVisitor::handleInnerBorrow.
…me completion Since partial_apply [on_stack] is considered as a BorrowingOperand now, and BorrowingOperand::visitScopeEndingUses can be called on the its forwarded values lifetime ends, update the assertion to reflect this.
Before move-checking values, complete the lifetimes of all the values derived from them via copy, borrow, and move. Collect all such values and their derived transitive values and then complete the lifetimes of each, visiting the instructions which produce them in post-order. Once OSSALifetimeCommpletion runs as part of SILGenCleanup, this code can be deleted.
nate-chandler
force-pushed
the
cherrypick/release/5.9/gh68328
branch
from
ea5af4f to
2bad5c7
Compare
|
@swift-ci please test |
tbkka
approved these changes
Sep 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: Fix wrong move-only diagnostic in no-return functions.
The move-checker for objects relies on a utility (
CanonicalizeOSSALifetime) to eliminate spurious copies. That utility is permitted to bail out in the face of SIL it doesn't understand. In particular, the utility bails out if lifetimes are not complete; such lifetimes occur in no-return functions. The checker, however, just issues a diagnosticUsage of a noncopyable type that compiler can't verify. This is a compiler bug.in the face of that bail out.Since the move-checker relies on the utility not bailing out, the fix is for the move-checker to transform the SIL before using the utility--specifically, to complete OSSA lifetimes via
OSSALifetimeCompletion. This must be done for each value which will be checked and values derived from it via ownership and forwarding operations.This is the first use of
OSSALifetimeCompletionon release/5.9. The utility is, however, already in use on main in TempRValueElimination and Mem2Reg. As a result, several fixes from main need to be picked up on release/5.9.Risk: Low. This only affects functions with move-only values, but the fix is to use the
OSSALifetimeCompletionutility on release/5.9 for the first time.Scope: Narrow. Only functions with move-only values are affected.
Original PR: #68342
Reviewed By: Meghana Gupta ( @meg-gupta )
Testing: Added test.
Resolves: rdar://115185992