Skip to content

Commit 6bafc84

Browse files
authored
Remove end_lifetime being considered as an end of scope marker (swiftlang#38851)
OSSA rauw cleans up end of scope markers before rauw'ing. This can lead to inadvertant deleting of end_lifetime, later resulting in an ownership verifier error indicating a leak. This PR stops treating end_lifetime scope ending like end_borrow/end_access.
1 parent b651278 commit 6bafc84

File tree

6 files changed

+50
-13
lines changed

6 files changed

+50
-13
lines changed

include/swift/SILOptimizer/Utils/InstOptUtils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ NullablePtr<SILInstruction> createDecrementBefore(SILValue ptr,
7070
Optional<SILBasicBlock::iterator> getInsertAfterPoint(SILValue val);
7171

7272
/// True if this instruction's only uses are debug_value (in -O mode),
73-
/// destroy_value, or end-of-scope instruction such as end_borrow.
74-
bool hasOnlyEndOfScopeOrDestroyUses(SILInstruction *inst);
73+
/// destroy_value, end_lifetime or end-of-scope instruction such as end_borrow.
74+
bool hasOnlyEndOfScopeOrEndOfLifetimeUses(SILInstruction *inst);
7575

7676
/// Return the number of @inout arguments passed to the given apply site.
7777
unsigned getNumInOutArguments(FullApplySite applySite);

lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,13 @@ bool swift::isEndOfScopeMarker(SILInstruction *user) {
280280
return false;
281281
case SILInstructionKind::EndAccessInst:
282282
case SILInstructionKind::EndBorrowInst:
283-
case SILInstructionKind::EndLifetimeInst:
284283
return true;
285284
}
286285
}
287286

288287
bool swift::isIncidentalUse(SILInstruction *user) {
289288
return isEndOfScopeMarker(user) || user->isDebugInstruction() ||
290-
isa<FixLifetimeInst>(user);
289+
isa<FixLifetimeInst>(user) || isa<EndLifetimeInst>(user);
291290
}
292291

293292
bool swift::onlyAffectsRefCount(SILInstruction *user) {

lib/SILOptimizer/Transforms/CopyPropagation.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ void CopyPropagation::run() {
488488
// Dead borrow scopes must be removed as uses before canonicalizing the
489489
// outer copy.
490490
if (auto *beginBorrow = dyn_cast<BeginBorrowInst>(borrow.value)) {
491-
if (hasOnlyEndOfScopeOrDestroyUses(beginBorrow)) {
491+
if (hasOnlyEndOfScopeOrEndOfLifetimeUses(beginBorrow)) {
492492
deleter.recursivelyDeleteUsersIfDead(beginBorrow);
493493
}
494494
}

lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,13 @@ bool swift::isIntermediateRelease(SILInstruction *inst,
211211
return false;
212212
}
213213

214-
bool swift::hasOnlyEndOfScopeOrDestroyUses(SILInstruction *inst) {
214+
bool swift::hasOnlyEndOfScopeOrEndOfLifetimeUses(SILInstruction *inst) {
215215
for (SILValue result : inst->getResults()) {
216216
for (Operand *use : result->getUses()) {
217217
SILInstruction *user = use->getUser();
218218
bool isDebugUser = user->isDebugInstruction();
219-
if (!isa<DestroyValueInst>(user) && !isEndOfScopeMarker(user) &&
220-
!isDebugUser)
219+
if (!isa<DestroyValueInst>(user) && !isa<EndLifetimeInst>(user) &&
220+
!isEndOfScopeMarker(user) && !isDebugUser)
221221
return false;
222222
// Include debug uses only in Onone mode.
223223
if (isDebugUser && inst->getFunction()->getEffectiveOptimizationMode() <=
@@ -306,7 +306,7 @@ static bool isScopeAffectingInstructionDead(SILInstruction *inst,
306306
}
307307
// If the instruction has any use other than end of scope use or destroy_value
308308
// use, bail out.
309-
if (!hasOnlyEndOfScopeOrDestroyUses(inst)) {
309+
if (!hasOnlyEndOfScopeOrEndOfLifetimeUses(inst)) {
310310
return false;
311311
}
312312
// If inst is a copy or beginning of scope, inst is dead, since we know that
@@ -468,8 +468,8 @@ void InstructionDeleter::deleteWithUses(SILInstruction *inst, bool fixLifetimes,
468468
auto uses = llvm::to_vector<4>(result->getUses());
469469
for (Operand *use : uses) {
470470
SILInstruction *user = use->getUser();
471-
assert(forceDeleteUsers || isIncidentalUse(user)
472-
|| isa<DestroyValueInst>(user));
471+
assert(forceDeleteUsers || isIncidentalUse(user) ||
472+
isa<DestroyValueInst>(user));
473473
assert(!isa<BranchInst>(user) && "can't delete phis");
474474

475475
toDeleteInsts.push_back(user);
@@ -574,7 +574,7 @@ void InstructionDeleter::recursivelyForceDeleteUsersAndFixLifetimes(
574574
if (isIncidentalUse(inst) || isa<DestroyValueInst>(inst)) {
575575
forceDelete(inst);
576576
return;
577-
}
577+
}
578578
forceDeleteAndFixLifetimes(inst);
579579
}
580580

test/SILOptimizer/cse_ossa.sil

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ sil_stage canonical
44
import Builtin
55
import Swift
66

7-
class B { }
7+
class B {
8+
func foo()
9+
}
810
class E : B { }
911

1012
class C {}
@@ -1298,3 +1300,20 @@ bb0(%0 : $LazyKlass):
12981300
return %4 : $(Int64, Int64)
12991301
}
13001302

1303+
// CHECK-LABEL: sil [ossa] @test_end_lifetime :
1304+
// CHECK: upcast
1305+
// CHECK-NOT: upcast
1306+
// CHECK: } // end sil function 'test_end_lifetime'
1307+
sil [ossa] @test_end_lifetime : $@convention(method) (@owned E) -> () {
1308+
bb0(%0 : @owned $E):
1309+
%copy = copy_value %0 : $E
1310+
%u1 = upcast %0 : $E to $B
1311+
%f = objc_method %u1 : $B, #B.foo!foreign : (B) -> () -> (), $@convention(objc_method) (B) -> ()
1312+
%c1 = apply %f(%u1) : $@convention(objc_method) (B) -> ()
1313+
end_lifetime %u1 : $B
1314+
%u2 = upcast %copy : $E to $B
1315+
%c2 = apply %f(%u2) : $@convention(objc_method) (B) -> ()
1316+
end_lifetime %u2 : $B
1317+
%r = tuple ()
1318+
return %r : $()
1319+
}

test/SILOptimizer/load_borrow_verify.sil

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,22 @@ bb0(%0 : @owned $K):
134134
%r = tuple ()
135135
return %r : $()
136136
}
137+
138+
sil [ossa] @test_end_lifetime : $@convention(thin) (@owned K) -> () {
139+
bb0(%0 : @owned $K):
140+
%1 = begin_borrow %0 : $K
141+
%2 = ref_element_addr %1 : $K, #K.x
142+
%3 = begin_access [read] [dynamic] %2 : $*B
143+
%4 = load_borrow %3 : $*B
144+
end_borrow %4 : $B
145+
end_access %3 : $*B
146+
%cast = unchecked_ref_cast %1 : $K to $Builtin.NativeObject
147+
%conv1 = unchecked_ownership_conversion %cast : $Builtin.NativeObject, @guaranteed to @owned
148+
end_borrow %1 : $K
149+
end_lifetime %0 : $K
150+
%conv2 = unchecked_ref_cast %conv1 : $Builtin.NativeObject to $K
151+
dealloc_ref %conv2 : $K
152+
%r = tuple ()
153+
return %r : $()
154+
}
155+

0 commit comments

Comments
 (0)