Skip to content

Commit 566844e

Browse files
committed
Fix ClosureSpecialize. Don't insert releases in readonly functions.
For now, disable specialization when it would result in adding releases to readnone, readonly, or releasenone functions. Fixes rdar://105887096 TODO: A @NoEscape closure should never be converted to an @owned argument regardless of the function attribute.
1 parent 8862b1b commit 566844e

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

lib/SILOptimizer/IPO/ClosureSpecializer.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,18 @@ bool SILClosureSpecializerTransform::gatherCallSites(
13461346
continue;
13471347
}
13481348

1349+
// Specializing a readnone, readonly, releasenone function with a
1350+
// nontrivial context is illegal. Inserting a release in such a function
1351+
// results in miscompilation after other optimizations.
1352+
// For now, the specialization is disabled.
1353+
//
1354+
// TODO: A @noescape closure should never be converted to an @owned
1355+
// argument regardless of the function attribute.
1356+
if (!OnlyHaveThinToThickClosure
1357+
&& ApplyCallee->getEffectsKind() <= EffectsKind::ReleaseNone) {
1358+
continue;
1359+
}
1360+
13491361
// Avoid an infinite specialization loop caused by repeated runs of
13501362
// ClosureSpecializer and CapturePropagation.
13511363
// CapturePropagation propagates constant function-literals. Such

test/SILOptimizer/closure_specialize_attrs.sil

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ class C {}
66

77
sil [ossa] @getC : $@convention(thin) () -> @owned C
88

9+
class Storage {}
10+
11+
struct Val {}
12+
913
// Verify that the argument to the specialized take_closure is still @_eagerMove.
1014

1115
// CHECK-LABEL: sil {{.*}}@$s12take_closure0B04main1CCTf1nc_n : {{.*}}{
@@ -44,3 +48,40 @@ bb0(%0 : $C):
4448
%retval = tuple()
4549
return %retval : $()
4650
}
51+
52+
// =============================================================================
53+
// rdar://105887096: do not insert a retain inside a read-only function.
54+
// For now, the specialization is disabled.
55+
//
56+
// TODO: A @noescape closure should never be converted to an @owned argument
57+
// regardless of the function attribute.
58+
59+
// This should not be specialized until we support guaranteed arguments.
60+
// CHECK-NOT: @$s20takesReadOnlyClosure
61+
sil private [readonly] @takesReadOnlyClosure : $@convention(thin) (@noescape @callee_guaranteed (Val) -> Val) -> Val {
62+
bb0(%2 : $@noescape @callee_guaranteed (Val) -> Val):
63+
%46 = struct $Val ()
64+
%261 = apply %2(%46) : $@noescape @callee_guaranteed (Val) -> Val
65+
return %261 : $Val
66+
}
67+
68+
sil private @readOnlyClosure : $@convention(thin) (Val, @guaranteed Storage) -> Val {
69+
bb0(%0 : $Val, %1 : @closureCapture $Storage):
70+
%46 = struct $Val ()
71+
return %46 : $Val
72+
}
73+
74+
// CHECK-LABEL: sil @testPassReadOnlyClosure : $@convention(method) (@guaranteed Storage) -> Val {
75+
// CHECK-NOT: @owned Storage
76+
// CHECK: apply %{{.*}} : $@convention(thin) (@noescape @callee_guaranteed (Val) -> Val) -> Val
77+
// CHECK-LABEL: } // end sil function 'testPassReadOnlyClosure'
78+
sil @testPassReadOnlyClosure : $@convention(method) (@guaranteed Storage) -> Val {
79+
bb0(%0 : $Storage):
80+
%176 = function_ref @readOnlyClosure : $@convention(thin) (Val, @guaranteed Storage) -> Val
81+
%177 = partial_apply [callee_guaranteed] [on_stack] %176(%0) : $@convention(thin) (Val, @guaranteed Storage) -> Val
82+
%178 = mark_dependence %177 : $@noescape @callee_guaranteed (Val) -> Val on %0 : $Storage
83+
%188 = function_ref @takesReadOnlyClosure : $@convention(thin) (@noescape @callee_guaranteed (Val) -> Val) -> Val
84+
%189 = apply %188(%178) : $@convention(thin) (@noescape @callee_guaranteed (Val) -> Val) -> Val
85+
dealloc_stack %177 : $@noescape @callee_guaranteed (Val) -> Val
86+
return %189 : $Val
87+
}

0 commit comments

Comments
 (0)