Skip to content

Commit ad02d91

Browse files
authored
Fix a minor issue in ExistentialSpecializer for OSSA (swiftlang#36180)
While cloning arguments for existential specializer, do not create copy for object types. Currently, for guaranteed parameter types it unnecessarily creates a copy and cleans it up with a destroy. The discrepency is seen when cloning an instruction like open_existential_ref which was previously using guaranteed operand and had guaranteed forwarding ownership, is now replaced to have the owned copy as its operand during cloning.
1 parent b676f29 commit ad02d91

File tree

2 files changed

+50
-18
lines changed

2 files changed

+50
-18
lines changed

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialTransform.cpp

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,8 @@ void ExistentialSpecializerCloner::cloneAndPopulateFunction() {
108108
// A return location can't be used for a non-return instruction.
109109
auto loc = RegularLocation::getAutoGeneratedLocation();
110110
for (SILValue cleanupVal : CleanupValues) {
111-
if (cleanupVal.getOwnershipKind() == OwnershipKind::Guaranteed) {
112-
Builder.emitEndBorrowOperation(loc, cleanupVal);
113-
} else {
114-
Builder.emitDestroyOperation(loc, cleanupVal);
115-
}
111+
assert(cleanupVal.getOwnershipKind() != OwnershipKind::Guaranteed);
112+
Builder.emitDestroyOperation(loc, cleanupVal);
116113
}
117114

118115
for (auto *ASI : llvm::reverse(AllocStackInsts))
@@ -240,10 +237,6 @@ void ExistentialSpecializerCloner::cloneArguments(
240237
}
241238
NewArgValue =
242239
NewFBuilder.emitLoadValueOperation(InsertLoc, NewArg, qual);
243-
} else {
244-
if (NewFBuilder.hasOwnership() && !origConsumed) {
245-
NewArgValue = NewFBuilder.emitCopyValueOperation(InsertLoc, NewArg);
246-
}
247240
}
248241

249242
/// Simple case: Create an init_existential.
@@ -262,15 +255,6 @@ void ExistentialSpecializerCloner::cloneArguments(
262255
StoreOwnershipQualifier::Init);
263256
InitRef = alloc;
264257
AllocStackInsts.push_back(alloc);
265-
} else {
266-
// Otherwise in ossa, we need to add init existential ref as something
267-
// to be cleaned up. In non-ossa, we do not insert the copies, so we do
268-
// not need to do it then.
269-
//
270-
// TODO: This would be simpler if we had managed value/cleanup scopes.
271-
if (NewFBuilder.hasOwnership() && !origConsumed) {
272-
CleanupValues.push_back(InitRef);
273-
}
274258
}
275259

276260
entryArgs.push_back(InitRef);

test/SILOptimizer/existential_transform_extras_ossa.sil

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,26 @@ internal class Klass2 : P {
2020
init()
2121
}
2222

23+
internal class Klass3 {
24+
}
25+
26+
extension Klass3 : P {
27+
@inline(never) func foo() -> Int32
28+
}
29+
30+
internal class Klass4 : Klass3 {
31+
func foo() -> Int {
32+
return 10
33+
}
34+
}
35+
36+
sil private [transparent] [thunk] @Klass4foo : $@convention(witness_method: P) (@in_guaranteed Klass4) -> Int32 {
37+
bb0(%0 : $*Klass4):
38+
%1 = integer_literal $Builtin.Int32, 10
39+
%2 = struct $Int32 (%1 : $Builtin.Int32)
40+
return %2 : $Int32
41+
}
42+
2343
@inline(never) internal func wrap_foo_ncp(a: inout P, b: inout P) -> Int32
2444

2545
@inline(never) func ncp()
@@ -264,3 +284,31 @@ bb0(%0 : @guaranteed $PlotView):
264284
%v = tuple ()
265285
return %v : $()
266286
}
287+
288+
// CHECK-LABEL: sil shared [ossa] @$s35testExistentialSpecializeGuaranteedTf4e_n :
289+
// CHECK: [[VAL1:%.*]] = init_existential_ref %0
290+
// CHECK: [[VAL2:%.*]] = open_existential_ref [[VAL1]]
291+
// CHECK-LABEL: } // end sil function '$s35testExistentialSpecializeGuaranteedTf4e_n'
292+
sil hidden [ossa] @testExistentialSpecializeGuaranteed : $@convention(thin) (@guaranteed Klass3 & P) -> Int32 {
293+
bb0(%0 : @guaranteed $Klass3 & P):
294+
%2 = open_existential_ref %0 : $Klass3 & P to $@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P
295+
%3 = alloc_stack $@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P
296+
%4 = store_borrow %2 to %3 : $*@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P
297+
%5 = witness_method $@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P, #P.foo : <Self where Self : P> (Self) -> () -> Int32, %2 : $@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
298+
%6 = apply %5<@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P>(%3) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int32
299+
dealloc_stack %3 : $*@opened("77949BFA-77BC-11EB-BC0E-F2189810406F") Klass3 & P
300+
return %6 : $Int32
301+
}
302+
303+
sil hidden [ossa] @entrypoint : $@convention(thin) (@guaranteed Klass4) -> Int32 {
304+
bb0(%0 : @guaranteed $Klass4):
305+
%1 = init_existential_ref %0 : $Klass4 : $Klass4, $Klass3 & P
306+
%func = function_ref @testExistentialSpecializeGuaranteed : $@convention(thin) (@guaranteed Klass3 & P) -> Int32
307+
%res = apply %func(%1) : $@convention(thin) (@guaranteed Klass3 & P) -> Int32
308+
return %res : $Int32
309+
}
310+
311+
sil_witness_table hidden Klass4: P module devirtualize_protocol_composition {
312+
method #P.foo: <Self where Self : P> (Self) -> () -> Int32 : @Klass4foo
313+
}
314+

0 commit comments

Comments
 (0)