Skip to content

Commit 9383df8

Browse files
committed
AllocStackToBox: fix a bug which results in a too early released captured variable
In case of a borrowed `alloc_box`, the optimization didn't look through the `begin_borrow` when calculating the final release of the box. This resulted in inserting the destroy of the inserted `alloc_stack` too early. rdar://97087762
1 parent 34338b9 commit 9383df8

File tree

2 files changed

+41
-4
lines changed

2 files changed

+41
-4
lines changed

lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ static llvm::cl::opt<bool> AllocBoxToStackAnalyzeApply(
5353
// SIL Utilities for alloc_box Promotion
5454
//===----------------------------------------------------------------------===//
5555

56-
static SILValue stripOffCopyValue(SILValue V) {
57-
while (auto *CVI = dyn_cast<CopyValueInst>(V)) {
58-
V = CVI->getOperand();
56+
static SILValue stripOffCopyAndBorrow(SILValue V) {
57+
while (isa<CopyValueInst>(V) || isa<BeginBorrowInst>(V)) {
58+
V = cast<SingleValueInstruction>(V)->getOperand(0);
5959
}
6060
return V;
6161
}
@@ -127,7 +127,7 @@ static bool addLastRelease(SILValue V, SILBasicBlock *BB,
127127
for (auto I = BB->rbegin(); I != BB->rend(); ++I) {
128128
if (isa<StrongReleaseInst>(*I) || isa<DeallocBoxInst>(*I) ||
129129
isa<DestroyValueInst>(*I)) {
130-
if (stripOffCopyValue(I->getOperand(0)) != V)
130+
if (stripOffCopyAndBorrow(I->getOperand(0)) != V)
131131
continue;
132132

133133
Releases.push_back(&*I);

test/SILOptimizer/allocbox_to_stack_ownership.sil

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,43 @@ bb0(%0 : $Int):
469469
unreachable
470470
}
471471

472+
sil [ossa] @closureWithBoxArg : $@convention(thin) (@guaranteed { var SomeClass }) -> () {
473+
bb0(%0 : @guaranteed ${ var SomeClass }):
474+
%r = tuple ()
475+
return %r : $()
476+
}
477+
478+
sil [ossa] @useClosure : $@convention(thin) (@guaranteed @callee_guaranteed () -> ()) -> () {
479+
bb0(%0 : @guaranteed $@callee_guaranteed () -> ()):
480+
%r = tuple ()
481+
return %r : $()
482+
}
483+
484+
// CHECK-LABEL: sil [ossa] @finalReleaseWithBorrow
485+
// CHECK: [[S:%[0-9]+]] = alloc_stack {{.*}}$SomeClass
486+
// CHECK-NOT: destroy_addr
487+
// CHECK: [[F:%[0-9]+]] = function_ref @useClosure
488+
// CHECK: apply [[F]]
489+
// CHECK: destroy_addr [[S]]
490+
// CHECK: } // end sil function 'finalReleaseWithBorrow'
491+
sil [ossa] @finalReleaseWithBorrow : $@convention(thin) (@owned SomeClass) -> () {
492+
bb0(%0 : @owned $SomeClass):
493+
%1 = alloc_box $ { var SomeClass }
494+
%2 = begin_borrow %1 : ${ var SomeClass }
495+
%1a = project_box %2 : ${ var SomeClass }, 0
496+
store %0 to [init] %1a : $*SomeClass
497+
%3 = function_ref @closureWithBoxArg : $@convention(thin) (@guaranteed { var SomeClass }) -> ()
498+
%4 = copy_value %2 : ${ var SomeClass }
499+
%5 = partial_apply [callee_guaranteed] %3(%4) : $@convention(thin) (@guaranteed { var SomeClass }) -> ()
500+
end_borrow %2 : ${ var SomeClass }
501+
destroy_value %1 : ${ var SomeClass }
502+
%6 = function_ref @useClosure : $@convention(thin) (@guaranteed @callee_guaranteed () -> ()) -> ()
503+
apply %6(%5) : $@convention(thin) (@guaranteed @callee_guaranteed () -> ()) -> ()
504+
destroy_value %5 : $@callee_guaranteed () -> ()
505+
%10 = tuple ()
506+
return %10 : $()
507+
}
508+
472509
// CHECK-LABEL: sil [transparent] [serialized] [ossa] @mightApply : $@convention(thin) <U where U : P> (@owned @callee_owned () -> @out U) -> ()
473510
sil [transparent] [serialized] [ossa] @mightApply : $@convention(thin) <U where U : P> (@owned @callee_owned () -> @out U) -> () {
474511
// CHECK: bb0

0 commit comments

Comments
 (0)