Skip to content

Commit 8c5737d

Browse files
gottesmmgottesmm
committed
[ownership] Change thin_to_thick function to always produce a none value.
It can already only accept values with none ownership and the merging of ownership around ownership phis ensure that if we phi this with a partial_apply or the like, we get the appropriate ownership on any such ownership phi values. We are now out of SILGen emitting fewer destroy_value unnecessarily on thin_to_thick functions. This changed some codegen and also forced me to update some tests/fix AutoDiff. I also deleted the DebugInfo test mandatoryinlining-wrongdebugscope.swift since: 1. It was depending on these destroys being there. 2. Given the need to improve the test @aprantl suggested I just eliminate it solving the test failure for me.
1 parent 303d88c commit 8c5737d

File tree

12 files changed

+122
-65
lines changed

12 files changed

+122
-65
lines changed

‎include/swift/SIL/SILInstruction.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4787,13 +4787,14 @@ class Name##ToRefInst \
47874787
class ThinToThickFunctionInst final
47884788
: public UnaryInstructionWithTypeDependentOperandsBase<
47894789
SILInstructionKind::ThinToThickFunctionInst, ThinToThickFunctionInst,
4790-
ConversionInst> {
4790+
OwnershipForwardingConversionInst> {
47914791
friend SILBuilder;
47924792

47934793
ThinToThickFunctionInst(SILDebugLocation DebugLoc, SILValue Operand,
47944794
ArrayRef<SILValue> TypeDependentOperands, SILType Ty)
47954795
: UnaryInstructionWithTypeDependentOperandsBase(
4796-
DebugLoc, Operand, TypeDependentOperands, Ty) {}
4796+
DebugLoc, Operand, TypeDependentOperands, Ty,
4797+
Operand.getOwnershipKind()) {}
47974798

47984799
static ThinToThickFunctionInst *
47994800
create(SILDebugLocation DebugLoc, SILValue Operand, SILType Ty,

‎lib/SIL/IR/ValueOwnership.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ CONSTANT_OWNERSHIP_INST(Unowned, ObjCProtocol)
150150
CONSTANT_OWNERSHIP_INST(Unowned, ValueToBridgeObject)
151151
CONSTANT_OWNERSHIP_INST(None, GetAsyncContinuation)
152152
CONSTANT_OWNERSHIP_INST(Unowned, GetAsyncContinuationAddr)
153+
CONSTANT_OWNERSHIP_INST(None, ThinToThickFunction)
153154
#undef CONSTANT_OWNERSHIP_INST
154155

155156
#define CONSTANT_OR_NONE_OWNERSHIP_INST(OWNERSHIP, INST) \
@@ -206,9 +207,6 @@ CONSTANT_OR_NONE_OWNERSHIP_INST(Owned, MarkUninitialized)
206207
// a performance perspective.
207208
CONSTANT_OR_NONE_OWNERSHIP_INST(Unowned, UncheckedBitwiseCast)
208209

209-
// A thin_to_thick instruction can return a trivial (@noescape) type.
210-
CONSTANT_OR_NONE_OWNERSHIP_INST(Owned, ThinToThickFunction)
211-
212210
#undef CONSTANT_OR_NONE_OWNERSHIP_INST
213211

214212
// For a forwarding instruction, we loop over all operands and make sure that

‎lib/SILOptimizer/Mandatory/Differentiation.cpp‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,9 @@ static void copyParameterArgumentsForApply(
323323
// Copy the argument if it's to be owned by the newly created closure.
324324
// Objects are to be retained.
325325
if (arg->getType().isObject()) {
326-
auto newArg = copyBuilder.emitCopyValueOperation(loc, arg);
326+
auto newArg = arg;
327+
if (newArg.getOwnershipKind() != ValueOwnershipKind::None)
328+
newArg = copyBuilder.emitCopyValueOperation(loc, arg);
327329
collectNewArg(newArg);
328330
continue;
329331
}
@@ -498,8 +500,9 @@ emitDerivativeFunctionReference(
498500
builder.emitBeginBorrowOperation(original.getLoc(), original);
499501
SILValue derivativeFn = builder.createDifferentiableFunctionExtract(
500502
borrowedDiffFunc.getLoc(), kind, borrowedDiffFunc);
501-
derivativeFn =
502-
builder.emitCopyValueOperation(original.getLoc(), derivativeFn);
503+
if (derivativeFn.getOwnershipKind() != ValueOwnershipKind::None)
504+
derivativeFn =
505+
builder.emitCopyValueOperation(original.getLoc(), derivativeFn);
503506
builder.emitEndBorrowOperation(original.getLoc(), borrowedDiffFunc);
504507
return std::make_pair(derivativeFn, desiredIndices);
505508
}
@@ -1208,9 +1211,11 @@ SILValue DifferentiationTransformer::promoteToDifferentiableFunction(
12081211
for (auto *buf : llvm::reverse(newBuffersToDealloc))
12091212
builder.createDeallocStack(loc, buf);
12101213

1211-
auto origFnCopy = builder.emitCopyValueOperation(loc, origFnOperand);
1214+
// If our original copy does not have none ownership, copy it.
1215+
if (origFnOperand.getOwnershipKind() != ValueOwnershipKind::None)
1216+
origFnOperand = builder.emitCopyValueOperation(loc, origFnOperand);
12121217
auto *newDiffFn = context.createDifferentiableFunction(
1213-
builder, loc, parameterIndices, resultIndices, origFnCopy,
1218+
builder, loc, parameterIndices, resultIndices, origFnOperand,
12141219
std::make_pair(derivativeFns[0], derivativeFns[1]));
12151220
context.getDifferentiableFunctionInstWorklist().push_back(dfi);
12161221
return newDiffFn;
@@ -1223,7 +1228,8 @@ SILValue DifferentiationTransformer::promoteToLinearFunction(
12231228
// with an undef transpose function operand. Eventually, a legitimate
12241229
// transpose function operand should be created and used.
12251230
auto origFnOperand = lfi->getOriginalFunction();
1226-
auto origFnCopy = builder.emitCopyValueOperation(loc, origFnOperand);
1231+
if (origFnOperand.getOwnershipKind() != ValueOwnershipKind::None)
1232+
origFnOperand = builder.emitCopyValueOperation(loc, origFnOperand);
12271233
auto *parameterIndices = lfi->getParameterIndices();
12281234
auto originalType = origFnOperand->getType().castTo<SILFunctionType>();
12291235
auto transposeFnType = originalType->getAutoDiffTransposeFunctionType(
@@ -1232,7 +1238,7 @@ SILValue DifferentiationTransformer::promoteToLinearFunction(
12321238
auto transposeType = SILType::getPrimitiveObjectType(transposeFnType);
12331239
auto transposeFn = SILUndef::get(transposeType, builder.getFunction());
12341240
auto *newLinearFn = context.createLinearFunction(
1235-
builder, loc, parameterIndices, origFnCopy, SILValue(transposeFn));
1241+
builder, loc, parameterIndices, origFnOperand, SILValue(transposeFn));
12361242
context.getLinearFunctionInstWorklist().push_back(lfi);
12371243
return newLinearFn;
12381244
}

‎test/AutoDiff/SILGen/autodiff_builtins.swift‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,15 @@ func applyTranspose_f_direct_arity1(_ x: Float) -> Float {
9898
// CHECK: [[NOESC_LINEAR:%.*]] = convert_escape_to_noescape [not_guaranteed] [[LINEAR]] : $@differentiable(linear) @callee_guaranteed (Float) -> Float to $@differentiable(linear) @noescape @callee_guaranteed (Float) -> Float
9999
// CHECK: [[TRANS:%.*]] = linear_function_extract [transpose] [[NOESC_LINEAR]] : $@differentiable(linear) @noescape @callee_guaranteed (Float) -> Float
100100
// CHECK: [[RESULT:%.*]] = apply [[TRANS]]([[X]]) : $@noescape @callee_guaranteed (Float) -> Float
101-
// CHECK: destroy_value [[LINEAR]] : $@differentiable(linear) @callee_guaranteed (Float) -> Float
102101
// CHECK: return [[RESULT]] : $Float
102+
// CHECK: } // end sil function 'applyTranspose_f_direct_arity1'
103103

104104
@_silgen_name("applyTranspose_f_direct_arity2")
105105
func applyTranspose_f_direct_arity2(_ x: Float) -> (Float, Float) {
106106
return Builtin.applyTranspose_arity2(f_direct_arity2, x)
107107
}
108-
// CHECK-LABEL: sil{{.*}}@applyTranspose_f_direct_arity2
108+
109+
// CHECK-LABEL: sil{{.*}}@applyTranspose_f_direct_arity2 :
109110
// CHECK: bb0([[X:%.*]] : $Float)
110111
// CHECK: [[ORIG:%.*]] = function_ref @f_direct_arity2 : $@convention(thin) (Float, Float) -> Float
111112
// CHECK: [[THICK_ORIG:%.*]] = thin_to_thick_function [[ORIG]] : $@convention(thin) (Float, Float) -> Float to $@callee_guaranteed (Float, Float) -> Float
@@ -114,9 +115,9 @@ func applyTranspose_f_direct_arity2(_ x: Float) -> (Float, Float) {
114115
// CHECK: [[TRANS:%.*]] = linear_function_extract [transpose] [[NOESC_LINEAR]] : $@differentiable(linear) @noescape @callee_guaranteed (Float, Float) -> Float
115116
// CHECK: [[RESULT:%.*]] = apply [[TRANS]]([[X]]) : $@noescape @callee_guaranteed (Float) -> (Float, Float)
116117
// CHECK: ([[RES1:%.*]], [[RES2:%.*]]) = destructure_tuple [[RESULT]] : $(Float, Float)
117-
// CHECK: destroy_value [[LINEAR]] : $@differentiable(linear) @callee_guaranteed (Float, Float) -> Float
118118
// CHECK: [[RESULT:%.*]] = tuple ([[RES1]] : $Float, [[RES2]] : $Float)
119119
// CHECK: return [[RESULT]] : $(Float, Float)
120+
// CHECK: } // end sil function 'applyTranspose_f_direct_arity2'
120121

121122
@_silgen_name("applyTranspose_f_indirect_arity1")
122123
func applyTranspose_f_indirect_arity1<T: AdditiveArithmetic & Differentiable>(_ x: T) -> T {

‎test/AutoDiff/SILGen/reabstraction.swift‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,17 @@ func makeSignatureAbstract() {
2121

2222
// CHECK-LABEL: sil{{.*}}@makeSignatureAbstract
2323
// CHECK: [[BEFORE:%.*]] = differentiable_function [parameters 0] [results 0]
24-
// CHECK: [[BEFORE_BORROWED:%.*]] = begin_borrow [[BEFORE]]
25-
// CHECK: [[ORIG_0:%.*]] = differentiable_function_extract [original] [[BEFORE_BORROWED]]
24+
// CHECK: [[ORIG_0:%.*]] = differentiable_function_extract [original] [[BEFORE]]
2625
// CHECK: [[ORIG_1:%.*]] = copy_value [[ORIG_0]]
2726
// CHECK: [[ORIG_THUNK:%.*]] = function_ref {{.*}} : $@convention(thin) (@in_guaranteed Float, @guaranteed @callee_guaranteed (Float) -> Float) -> @out Float
2827
// CHECK: [[ORIG_2:%.*]] = partial_apply [callee_guaranteed] [[ORIG_THUNK]]([[ORIG_1]])
2928
// CHECK: [[ORIG_3:%.*]] = convert_function [[ORIG_2]]
30-
// CHECK: [[JVP_0:%.*]] = differentiable_function_extract [jvp] [[BEFORE_BORROWED]]
29+
// CHECK: [[JVP_0:%.*]] = differentiable_function_extract [jvp] [[BEFORE]]
3130
// CHECK: [[JVP_1:%.*]] = copy_value [[JVP_0]]
3231
// CHECK: [[JVP_THUNK:%.*]] = function_ref {{.*}} : $@convention(thin) (@in_guaranteed Float, @guaranteed @callee_guaranteed (Float) -> (Float, @owned @callee_guaranteed (Float) -> Float)) -> (@out Float, @owned @callee_guaranteed @substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Float, Float>)
3332
// CHECK: [[JVP_2:%.*]] = partial_apply [callee_guaranteed] [[JVP_THUNK]]([[JVP_1]])
3433
// CHECK: [[JVP_3:%.*]] = convert_function [[JVP_2]]
35-
// CHECK: [[VJP_0:%.*]] = differentiable_function_extract [vjp] [[BEFORE_BORROWED]]
34+
// CHECK: [[VJP_0:%.*]] = differentiable_function_extract [vjp] [[BEFORE]]
3635
// CHECK: [[VJP_1:%.*]] = copy_value [[VJP_0]]
3736
// CHECK: [[VJP_THUNK:%.*]] = function_ref {{.*}} : $@convention(thin) (@in_guaranteed Float, @guaranteed @callee_guaranteed (Float) -> (Float, @owned @callee_guaranteed (Float) -> Float)) -> (@out Float, @owned @callee_guaranteed @substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Float, Float>)
3837
// CHECK: [[VJP_2:%.*]] = partial_apply [callee_guaranteed] [[VJP_THUNK]]([[VJP_1]])

‎test/DebugInfo/mandatoryinlining-wrongdebugscope.swift‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎test/SIL/ownership-verifier/use_verifier.sil‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1328,3 +1328,92 @@ bb0(%0 : @guaranteed $GenericS<T>):
13281328
%2 = copy_value %1 : $GenericS<AnyObject>
13291329
return %2 : $GenericS<AnyObject>
13301330
}
1331+
1332+
/////////////////////////////
1333+
// Thin To Thick Ownership //
1334+
/////////////////////////////
1335+
1336+
// Make sure that we always have thin_to_thick_function produce any ownership
1337+
// and that ownership phis propagate things correctly.
1338+
1339+
sil @thin_to_thick_function_callee : $@convention(thin) () -> ()
1340+
1341+
sil [ossa] @thin_to_thick_function_noneownership_test_1 : $@convention(thin) () -> () {
1342+
bb0:
1343+
%f1 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1344+
%f2 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1345+
cond_br undef, bb1, bb2
1346+
1347+
bb1:
1348+
%f1thick = thin_to_thick_function %f1 : $@convention(thin) () -> () to $@callee_owned () -> ()
1349+
br bb3(%f1thick : $@callee_owned () -> ())
1350+
1351+
bb2:
1352+
%f2thick = thin_to_thick_function %f2 : $@convention(thin) () -> () to $@callee_owned () -> ()
1353+
br bb3(%f2thick : $@callee_owned () -> ())
1354+
1355+
bb3(%fUnknown : @owned $@callee_owned () -> ()):
1356+
destroy_value %fUnknown : $@callee_owned () -> ()
1357+
%9999 = tuple()
1358+
return %9999 : $()
1359+
}
1360+
1361+
sil [ossa] @thin_to_thick_function_noneownership_test_2 : $@convention(thin) () -> () {
1362+
bb0:
1363+
%f1 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1364+
%f2 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1365+
cond_br undef, bb1, bb2
1366+
1367+
bb1:
1368+
%f1thick = thin_to_thick_function %f1 : $@convention(thin) () -> () to $@callee_owned () -> ()
1369+
br bb3(%f1thick : $@callee_owned () -> ())
1370+
1371+
bb2:
1372+
%f2thick = thin_to_thick_function %f2 : $@convention(thin) () -> () to $@callee_owned () -> ()
1373+
br bb3(%f2thick : $@callee_owned () -> ())
1374+
1375+
bb3(%fUnknown : $@callee_owned () -> ()):
1376+
%9999 = tuple()
1377+
return %9999 : $()
1378+
}
1379+
1380+
sil [ossa] @thin_to_thick_function_noneownership_test_3 : $@convention(thin) () -> () {
1381+
bb0:
1382+
%f1 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1383+
%f2 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1384+
cond_br undef, bb1, bb2
1385+
1386+
bb1:
1387+
%f1thick = thin_to_thick_function %f1 : $@convention(thin) () -> () to $@callee_owned () -> ()
1388+
br bb3(%f1thick : $@callee_owned () -> ())
1389+
1390+
bb2:
1391+
%f2thick = thin_to_thick_function %f2 : $@convention(thin) () -> () to $@callee_owned () -> ()
1392+
br bb3(%f2thick : $@callee_owned () -> ())
1393+
1394+
bb3(%fUnknown : @guaranteed $@callee_owned () -> ()):
1395+
end_borrow %fUnknown : $@callee_owned () -> ()
1396+
%9999 = tuple()
1397+
return %9999 : $()
1398+
}
1399+
1400+
sil [ossa] @thin_to_thick_function_noneownership_test_4 : $@convention(thin) () -> () {
1401+
bb0:
1402+
%f1 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1403+
%f2 = function_ref @thin_to_thick_function_callee : $@convention(thin) () -> ()
1404+
cond_br undef, bb1, bb2
1405+
1406+
bb1:
1407+
%f1thick = thin_to_thick_function %f1 : $@convention(thin) () -> () to $@callee_owned () -> ()
1408+
br bb3(%f1thick : $@callee_owned () -> ())
1409+
1410+
bb2:
1411+
%f2thick = partial_apply %f2() : $@convention(thin) () -> ()
1412+
br bb3(%f2thick : $@callee_owned () -> ())
1413+
1414+
// This case we can only pass owned b/c of the partial apply.
1415+
bb3(%fUnknown : @owned $@callee_owned () -> ()):
1416+
destroy_value %fUnknown : $@callee_owned () -> ()
1417+
%9999 = tuple()
1418+
return %9999 : $()
1419+
}

‎test/SILGen/cf_members.swift‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ public func foo(_ x: Double) {
3939
// CHECK: [[FN:%.*]] = function_ref @$s10cf_members3fooyySdFSo10IAMStruct1VSdcfu_ : $@convention(thin) (Double) -> Struct1
4040
// CHECK: [[A:%.*]] = thin_to_thick_function [[FN]]
4141
// CHECK: [[BORROWED_A:%.*]] = begin_borrow [[A]]
42-
// CHECK: [[A_COPY:%.*]] = copy_value [[BORROWED_A]]
43-
// CHECK: [[BORROWED_A2:%.*]] = begin_borrow [[A_COPY]]
4442
let a: (Double) -> Struct1 = Struct1.init(value:)
45-
// CHECK: apply [[BORROWED_A2]]([[X]])
46-
// CHECK: destroy_value [[A_COPY]]
43+
// CHECK: [[NEW_Z_VALUE:%.*]] = apply [[BORROWED_A]]([[X]])
4744
// CHECK: end_borrow [[BORROWED_A]]
45+
// CHECK: [[WRITE:%.*]] = begin_access [modify] [unknown] [[Z]]
46+
// CHECK: assign [[NEW_Z_VALUE]] to [[WRITE]]
47+
// CHECK: end_access [[WRITE]]
4848
z = a(x)
4949

5050
// TODO: Support @convention(c) references that only capture thin metatype
@@ -77,14 +77,12 @@ public func foo(_ x: Double) {
7777
z = c(x)
7878
// CHECK: [[THUNK:%.*]] = function_ref @$s10cf_members3fooyySdFSo10IAMStruct1VSdcADcfu2_ : $@convention(thin) (Struct1) -> @owned @callee_guaranteed (Double) -> Struct1
7979
// CHECK: [[THICK:%.*]] = thin_to_thick_function [[THUNK]]
80-
// CHECK: [[BORROW:%.*]] = begin_borrow [[THICK]]
81-
// CHECK: [[COPY:%.*]] = copy_value [[BORROW]]
8280
let d: (Struct1) -> (Double) -> Struct1 = Struct1.translate(radians:)
8381
// CHECK: [[READ:%.*]] = begin_access [read] [unknown] [[Z]] : $*Struct1
8482
// CHECK: [[ZVAL:%.*]] = load [trivial] [[READ]]
85-
// CHECK: [[BORROW_COPY:%.*]] = begin_borrow [[COPY]]
86-
// CHECK: apply [[BORROW_COPY]]([[ZVAL]])
87-
// CHECK: destroy_value [[COPY]]
83+
// CHECK: [[THICK_BORROW:%.*]] = begin_borrow [[THICK]]
84+
// CHECK: apply [[THICK_BORROW]]([[ZVAL]])
85+
// CHECK: end_borrow [[THICK_BORROW]]
8886
z = d(z)(x)
8987

9088
// TODO: If we implement SE-0042, this should thunk the value Struct1 param

‎test/SILGen/generic_closures.swift‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ func generic_nocapture_existential<T>(_ x: T, y: Concept) -> Bool {
7474

7575
// CHECK: [[FOO:%.*]] = function_ref @$s16generic_closures0A22_nocapture_existential{{.*}} : $@convention(thin) (@in_guaranteed Concept) -> Bool
7676
// CHECK: [[FOO_CLOSURE:%.*]] = thin_to_thick_function [[FOO]]
77-
// CHECK: destroy_value [[FOO_CLOSURE]]
7877
let _ = foo
7978

8079
// CHECK: [[FOO:%.*]] = function_ref @$s16generic_closures0A22_nocapture_existential{{.*}} : $@convention(thin) (@in_guaranteed Concept) -> Bool

‎test/SILGen/implicitly_unwrapped_optional.swift‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,8 @@ func sr3758() {
7070
// CHECK: [[CLOSURE:%.+]] = function_ref @$s29implicitly_unwrapped_optional6sr3758yyFyypSgcfU_ : $@convention(thin) (@in_guaranteed Optional<Any>) -> ()
7171
// CHECK: [[F:%.+]] = thin_to_thick_function [[CLOSURE]] : $@convention(thin) (@in_guaranteed Optional<Any>) -> () to $@callee_guaranteed (@in_guaranteed Optional<Any>) -> ()
7272
// CHECK: [[BORROWED_F:%.*]] = begin_borrow [[F]]
73-
// CHECK: [[CALLEE:%.+]] = copy_value [[BORROWED_F]] : $@callee_guaranteed (@in_guaranteed Optional<Any>) -> ()
74-
// CHECK: [[BORROWED_CALLEE:%.*]] = begin_borrow [[CALLEE]]
75-
// CHECK: = apply [[BORROWED_CALLEE]]({{%.+}}) : $@callee_guaranteed (@in_guaranteed Optional<Any>) -> ()
76-
// CHECK: end_borrow [[BORROWED_CALLEE]]
77-
// destroy_value [[CALLEE]]
73+
// CHECK: = apply [[BORROWED_F]]({{%.+}}) : $@callee_guaranteed (@in_guaranteed Optional<Any>) -> ()
7874
// CHECK: end_borrow [[BORROWED_F]]
79-
// CHECK: destroy_value [[F]]
8075
let f: ((Any?) -> Void) = { (arg: Any!) in }
8176
f(nil)
8277
} // CHECK: end sil function '$s29implicitly_unwrapped_optional6sr3758yyF'

0 commit comments

Comments
 (0)