[NFC] Add SILGenCleanup::completeOSSLifetimes #64022
Conversation
Add local lifetime-ending operations to any owned or borrowed value. This puts a single value into valid OSSA form so that linear lifetime checking will pass. Also adds UnreachableLifetimeCompletion which fixes OSSA after converting a code path to unreachable (e.g. DiagnoseUnreachable and MandatoryInlining).
Disabled by default during initial testing.
Add a separate 'verifyOwnership()' entry point so it's possible to check OSSA lifetimes at various points. Move SILGenCleanup into a SILGen pass pipeline. After SILGen, verify incomplete OSSA. After SILGenCleanup, verify ownership.
Use DeadEndBlocks to ignore missing end-borrows and destroys. Allow SILGen to be as sloppy as it wants with OSSA cleanups.
Presently under -enable-ossa-complete-lifetimes. This allows SILGen to skip OSSA cleanups, for example at dead-end blocks. Long term, we may remove OSSA cleanups from SILGen entirely (except for lexical borrow scopes). This changes lets us experiment with that option.
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci smoke test |
|
PR testing is blocked by |
| /// Interior liveness handles the following cases naturally: | ||
| /// | ||
| /// When completing the lifetime if the initial value, %v1, transitively | ||
| /// include all dominated reborrows. %phi1 in this example: |
|
|
||
| static SILInstruction *endOSSALifetime(SILValue value, SILBuilder &builder) { | ||
| auto loc = | ||
| RegularLocation::getAutoGeneratedLocation(builder.getInsertionPointLoc()); |
There was a problem hiding this comment.
CleanupLocation(RegularLocation::...)?
There was a problem hiding this comment.
We use RegularLocation::getAutoGeneratedLocation everywhere for compiler-generated destroys. I really don't know if CleanupLocation will work once we move the destroy somewhere else. CleanupLocation seems to be for proper cleanups generated by the frontend. (@adrian-prantl ). At any rate, if we're doing it wrong, we should probably change our approach everywhere. Until then, I'll be consistent.
There was a problem hiding this comment.
It looks like everywhere that's done we may run afoul of the debug scope hole checking. The code that checks for them https://github.com/apple/swift/blob/main/lib/SIL/Verifier/SILVerifier.cpp#L6216-L6217
// lib/SIL/Verifier/SILVerifier.cpp:6216
if (SI.getLoc().getKind() == SILLocation::CleanupKind)
continue;
does skip over instructions whose locations are cleanups but not those that are autogenerated. I've encountered this problem before: #62533 . More than once, actually: #40516 .
if CleanupLocation will work once we move the destroy somewhere else
To clarify, the suggestion is
CleanupLocation(RegularLocation::getAutoGeneratedLocation(builder.getInsertionPointLoc()))
which looks to have the same characteristics as RegularLocation::getAutoGeneratedLocation(builder.getInsertionPointLoc())--specifically, the autoGenerated field--but with the kind changed from RegularKind to CleanupKind.
Aside: if it turns out that creating a location that's both autogenerated and of cleanup kind is what we ought to be doing everywhere, a shorter spelling would be nice--like a getAutoGeneratedLocation on CleanupLocation.
Presently under -enable-ossa-complete-lifetimes.
This allows SILGen to skip OSSA cleanups, for example at dead-end
blocks.
Long term, we may remove OSSA cleanups from SILGen entirely (except
for lexical borrow scopes). This changes lets us experiment with that
option.