Skip to content

Commit 6191de6

Browse files
committed
[ClosureSpecializer] Don't release trivial noescape apply arg twice.
When a specialization is created, in the original function, releases are added in two different places: (1) `ClosureSpecCloner::populateCloned` (2) `rewriteApplyInst` In the former, releases are added for closures which are guaranteed or trivial noescape (but with owned convention). In the latter, releases are added for closures that are owned. Previously, when emitting releases at (2), whether the closure was trivial noescape wasn't considered. The result was inserting the releases twice, an overrelease. Here, fix (2) to recognize trivial noescape as not +1. rdar://110115795
1 parent 97daf4f commit 6191de6

File tree

2 files changed

+47
-1
lines changed

2 files changed

+47
-1
lines changed

lib/SILOptimizer/IPO/ClosureSpecializer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ static void rewriteApplyInst(const CallSiteDescriptor &CSDesc,
482482
// If we passed in the original closure as @owned, then insert a release
483483
// right after NewAI. This is to balance the +1 from being an @owned
484484
// argument to AI.
485-
if (CSDesc.isClosureConsumed() && CSDesc.closureHasRefSemanticContext())
485+
if (CSDesc.isClosureConsumed() && !CSDesc.isTrivialNoEscapeParameter() &&
486+
CSDesc.closureHasRefSemanticContext())
486487
Builder.createReleaseValue(Closure->getLoc(), Closure,
487488
Builder.getDefaultAtomicity());
488489

test/SILOptimizer/closure_specialize.sil

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,51 @@ bb0(%0 : $Int):
702702
return %empty : $()
703703
}
704704

705+
sil @testClosureConvertHelper3 : $@convention(thin) (Int) -> Int
706+
sil [reabstraction_thunk] @reabstractionThunk3 : $@convention(thin) (@noescape @callee_guaranteed () -> Int) -> @out Int
707+
708+
sil @testClosureThunkNoEscape3 : $@convention(thin) (@owned @noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>) -> @out () {
709+
entry(%empty : $*(), %closure : $@noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>):
710+
%out = alloc_stack $Int
711+
%ret = apply %closure(%out) : $@noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>
712+
dealloc_stack %out : $*Int
713+
store %ret to %empty : $*()
714+
%retval = tuple ()
715+
return %retval : $()
716+
}
717+
718+
// CHECK-LABEL: sil @reabstractionTest4 {{.*}} {
719+
// CHECK: [[HELPER:%[^,]+]] = function_ref @testClosureConvertHelper3
720+
// CHECK: [[SPECIALIZATION:%[^,]+]] = function_ref @$s25testClosureThunkNoEscape30aB14ConvertHelper3SiTf1nc_n
721+
// CHECK: [[CLOSURE:%[^,]+]] = partial_apply [callee_guaranteed] [[HELPER]]
722+
// CHECK: [[NOESCAPE_CLOSURE:%[^,]+]] = convert_escape_to_noescape [[CLOSURE]]
723+
// CHECK: apply [[SPECIALIZATION]]{{.*}}
724+
// CHECK: release_value [[CLOSURE]]
725+
// CHECK-NOT: release_value [[CLOSURE]]
726+
// CHECK: strong_release [[NOESCAPE_CLOSURE]]
727+
// CHECK-LABEL: } // end sil function 'reabstractionTest4'
728+
sil @reabstractionTest4 : $(Int) -> () {
729+
bb0(%value : $Int):
730+
%testThrowingClosureConvertHelper = function_ref @testClosureConvertHelper3 : $@convention(thin) (Int) -> Int
731+
%closure = partial_apply [callee_guaranteed] %testThrowingClosureConvertHelper(%value) : $@convention(thin) (Int) -> Int
732+
%noescapeClosure = convert_escape_to_noescape %closure : $@callee_guaranteed () -> Int to $@noescape @callee_guaranteed () -> Int
733+
%thunk = function_ref @reabstractionThunk3 : $@convention(thin) (@noescape @callee_guaranteed () -> Int) -> @out Int
734+
%appliedThunk = partial_apply [callee_guaranteed] [on_stack] %thunk(%noescapeClosure) : $@convention(thin) (@noescape @callee_guaranteed () -> Int) -> @out Int
735+
736+
%dependency = mark_dependence %appliedThunk : $@noescape @callee_guaranteed () -> @out Int on %noescapeClosure : $@noescape @callee_guaranteed () -> Int
737+
%generified = convert_function %dependency : $@noescape @callee_guaranteed () -> @out Int to $@noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>
738+
%test = function_ref @testClosureThunkNoEscape3 : $@convention(thin) (@owned @noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>) -> @out ()
739+
strong_retain %generified : $@noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>
740+
%out = alloc_stack $()
741+
%ret = apply %test(%out, %generified) : $@convention(thin) (@owned @noescape @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Int>) -> @out ()
742+
dealloc_stack %out : $*()
743+
release_value %closure : $@callee_guaranteed () -> Int
744+
strong_release %noescapeClosure : $@noescape @callee_guaranteed () -> Int
745+
dealloc_stack %appliedThunk : $@noescape @callee_guaranteed () -> @out Int
746+
%empty = tuple ()
747+
return %empty : $()
748+
}
749+
705750
sil @testThrowingClosureConvertHelper : $@convention(thin) (Int) -> (Int, @error any Error)
706751
sil [reabstraction_thunk] @reabstractionThunkThrowing : $@convention(thin) (@noescape @callee_guaranteed () -> (Int, @error any Error)) -> (@out Int, @error any Error)
707752

0 commit comments

Comments
 (0)