Skip to content

Commit c65c1b5

Browse files
eecksteinslavapestov
eeckstein
authored and
slavapestov
committed
Re-apply "SILGen: Only give bridging and re-abstraction thunks a generic signature if necessary"
The commit was reverted because of a regression in the Prototypes/CollectionTransformers test. I believe the root cause was an escape analysis bug, which is fixed in my previous commit.
1 parent 3ec1fe0 commit c65c1b5

File tree

6 files changed

+64
-40
lines changed

6 files changed

+64
-40
lines changed

‎lib/SILGen/SILGenBridging.cpp

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -350,22 +350,30 @@ ManagedValue SILGenFunction::emitFuncToBlock(SILLocation loc,
350350
blockInterfaceTy->getParameters().end(),
351351
std::back_inserter(params));
352352

353-
auto genericSig = F.getLoweredFunctionType()->getGenericSignature();
354-
auto genericEnv = F.getGenericEnvironment();
355353
auto extInfo =
356354
SILFunctionType::ExtInfo()
357355
.withRepresentation(SILFunctionType::Representation::CFunctionPointer);
358356

359-
// The block invoke function must be pseudogeneric. This should be OK for now
360-
// since a bridgeable function's parameters and returns should all be
361-
// trivially representable in ObjC so not need to exercise the type metadata.
362-
//
363-
// Ultimately we may need to capture generic parameters in block storage, but
364-
// that will require a redesign of the interface to support dependent-layout
365-
// context. Currently we don't capture anything directly into a block but a
366-
// Swift closure, but that's totally dumb.
367-
if (genericSig)
368-
extInfo = extInfo.withIsPseudogeneric();
357+
CanGenericSignature genericSig;
358+
GenericEnvironment *genericEnv = nullptr;
359+
ArrayRef<Substitution> subs;
360+
if (fnTy->hasArchetype() || blockTy->hasArchetype()) {
361+
genericSig = F.getLoweredFunctionType()->getGenericSignature();
362+
genericEnv = F.getGenericEnvironment();
363+
364+
subs = F.getForwardingSubstitutions();
365+
366+
// The block invoke function must be pseudogeneric. This should be OK for now
367+
// since a bridgeable function's parameters and returns should all be
368+
// trivially representable in ObjC so not need to exercise the type metadata.
369+
//
370+
// Ultimately we may need to capture generic parameters in block storage, but
371+
// that will require a redesign of the interface to support dependent-layout
372+
// context. Currently we don't capture anything directly into a block but a
373+
// Swift closure, but that's totally dumb.
374+
if (genericSig)
375+
extInfo = extInfo.withIsPseudogeneric();
376+
}
369377

370378
auto invokeTy =
371379
SILFunctionType::get(genericSig,
@@ -404,7 +412,7 @@ ManagedValue SILGenFunction::emitFuncToBlock(SILLocation loc,
404412

405413
auto stackBlock = B.createInitBlockStorageHeader(loc, storage, invokeFn,
406414
SILType::getPrimitiveObjectType(blockTy),
407-
getForwardingSubstitutions());
415+
subs);
408416

409417
// Copy the block so we have an independent heap object we can hand off.
410418
auto heapBlock = B.createCopyBlock(loc, stackBlock);
@@ -622,10 +630,16 @@ SILGenFunction::emitBlockToFunc(SILLocation loc,
622630
CanSILFunctionType substFnTy;
623631
SmallVector<Substitution, 4> subs;
624632

633+
auto genericEnv = F.getGenericEnvironment();
634+
625635
// Declare the thunk.
626636
auto blockTy = block.getType().castTo<SILFunctionType>();
637+
627638
auto thunkTy = buildThunkType(block, funcTy, substFnTy, subs);
628-
auto thunk = SGM.getOrCreateReabstractionThunk(F.getGenericEnvironment(),
639+
if (!thunkTy->isPolymorphic())
640+
genericEnv = nullptr;
641+
642+
auto thunk = SGM.getOrCreateReabstractionThunk(genericEnv,
629643
thunkTy,
630644
blockTy,
631645
funcTy,
@@ -634,7 +648,7 @@ SILGenFunction::emitBlockToFunc(SILLocation loc,
634648
// Build it if necessary.
635649
if (thunk->empty()) {
636650
SILGenFunction thunkSGF(SGM, *thunk);
637-
thunk->setGenericEnvironment(F.getGenericEnvironment());
651+
thunk->setGenericEnvironment(genericEnv);
638652
auto loc = RegularLocation::getAutoGeneratedLocation();
639653
buildBlockToFuncThunkBody(thunkSGF, loc, blockTy, funcTy);
640654
}

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2314,11 +2314,24 @@ CanSILFunctionType SILGenFunction::buildThunkType(
23142314
// on the result type.
23152315
assert(expectedType->getExtInfo().hasContext());
23162316

2317-
// Just use the generic signature from the context.
2318-
// This isn't necessarily optimal.
2319-
auto genericSig = F.getLoweredFunctionType()->getGenericSignature();
2320-
auto subsArray = F.getForwardingSubstitutions();
2321-
subs.append(subsArray.begin(), subsArray.end());
2317+
auto extInfo = expectedType->getExtInfo()
2318+
.withRepresentation(SILFunctionType::Representation::Thin);
2319+
2320+
// Use the generic signature from the context if the thunk involves
2321+
// generic parameters.
2322+
CanGenericSignature genericSig;
2323+
GenericEnvironment *genericEnv = nullptr;
2324+
if (expectedType->hasArchetype() || sourceType->hasArchetype()) {
2325+
genericSig = F.getLoweredFunctionType()->getGenericSignature();
2326+
genericEnv = F.getGenericEnvironment();
2327+
auto subsArray = F.getForwardingSubstitutions();
2328+
subs.append(subsArray.begin(), subsArray.end());
2329+
2330+
// If our parent function was pseudogeneric, this thunk must also be
2331+
// pseudogeneric, since we have no way to pass generic parameters.
2332+
if (F.getLoweredFunctionType()->isPseudogeneric())
2333+
extInfo = extInfo.withIsPseudogeneric();
2334+
}
23222335

23232336
// Add the function type as the parameter.
23242337
SmallVector<SILParameterInfo, 4> params;
@@ -2328,14 +2341,6 @@ CanSILFunctionType SILGenFunction::buildThunkType(
23282341
sourceType->getExtInfo().hasContext()
23292342
? DefaultThickCalleeConvention
23302343
: ParameterConvention::Direct_Unowned});
2331-
2332-
auto extInfo = expectedType->getExtInfo()
2333-
.withRepresentation(SILFunctionType::Representation::Thin);
2334-
2335-
// If our parent function was pseudogeneric, this thunk must also be
2336-
// pseudogeneric, since we have no way to pass generic parameters.
2337-
if (F.getLoweredFunctionType()->isPseudogeneric())
2338-
extInfo = extInfo.withIsPseudogeneric();
23392344

23402345
// Map the parameter and expected types out of context to get the interface
23412346
// type of the thunk.
@@ -2406,10 +2411,15 @@ static ManagedValue createThunk(SILGenFunction &gen,
24062411
// Declare the thunk.
24072412
SmallVector<Substitution, 4> substitutions;
24082413
CanSILFunctionType substFnType;
2414+
24092415
auto thunkType = gen.buildThunkType(fn, expectedType,
24102416
substFnType, substitutions);
2417+
auto genericEnv = gen.F.getGenericEnvironment();
2418+
if (!thunkType->isPolymorphic())
2419+
genericEnv = nullptr;
2420+
24112421
auto thunk = gen.SGM.getOrCreateReabstractionThunk(
2412-
gen.F.getGenericEnvironment(),
2422+
genericEnv,
24132423
thunkType,
24142424
fn.getType().castTo<SILFunctionType>(),
24152425
expectedType,
@@ -2418,7 +2428,7 @@ static ManagedValue createThunk(SILGenFunction &gen,
24182428
// Build it if necessary.
24192429
if (thunk->empty()) {
24202430
// Borrow the context archetypes from the enclosing function.
2421-
thunk->setGenericEnvironment(gen.F.getGenericEnvironment());
2431+
thunk->setGenericEnvironment(genericEnv);
24222432
SILGenFunction thunkSGF(gen.SGM, *thunk);
24232433
auto loc = RegularLocation::getAutoGeneratedLocation();
24242434
buildThunkBody(thunkSGF, loc,

‎test/SILGen/generic_closures.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ class NestedGeneric<U> {
109109
}
110110

111111
// CHECK-LABEL: sil hidden @_TFC16generic_closures13NestedGeneric20nested_reabstraction{{.*}}
112-
// CHECK: [[REABSTRACT:%.*]] = function_ref @_TTRG__rXFo___XFo_iT__iT__
113-
// CHECK: partial_apply [[REABSTRACT]]<U, T>
112+
// CHECK: [[REABSTRACT:%.*]] = function_ref @_TTRXFo___XFo_iT__iT__
113+
// CHECK: partial_apply [[REABSTRACT]]
114114
func nested_reabstraction<T>(_ x: T) -> Optionable<() -> ()> {
115115
return .some({})
116116
}
@@ -213,8 +213,8 @@ func outer_generic<T>(t: T, i: Int) {
213213
let _: () -> () = inner_generic_nocapture
214214
// CHECK: [[FN:%.*]] = function_ref @_TFF16generic_closures13outer_genericurFT1tx1iSi_T_L_23inner_generic_nocaptureu__rFT1uqd___qd__ : $@convention(thin) <τ_0_0><τ_1_0> (@in τ_1_0) -> @out τ_1_0
215215
// CHECK: [[CLOSURE:%.*]] = partial_apply [[FN]]<T, ()>() : $@convention(thin) <τ_0_0><τ_1_0> (@in τ_1_0) -> @out τ_1_0
216-
// CHECK: [[THUNK:%.*]] = function_ref @_TTRGrXFo_iT__iT__XFo___
217-
// CHECK: [[THUNK_CLOSURE:%.*]] = partial_apply [[THUNK]]<T>([[CLOSURE]])
216+
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo_iT__iT__XFo___
217+
// CHECK: [[THUNK_CLOSURE:%.*]] = partial_apply [[THUNK]]([[CLOSURE]])
218218
// CHECK: strong_release [[THUNK_CLOSURE]]
219219

220220
// CHECK: [[FN:%.*]] = function_ref @_TFF16generic_closures13outer_genericurFT1tx1iSi_T_L_23inner_generic_nocaptureu__rFT1uqd___qd__ : $@convention(thin) <τ_0_0><τ_1_0> (@in τ_1_0) -> @out τ_1_0
@@ -223,8 +223,8 @@ func outer_generic<T>(t: T, i: Int) {
223223

224224
// CHECK: [[FN:%.*]] = function_ref @_TFF16generic_closures13outer_genericurFT1tx1iSi_T_L_14inner_generic1u__rfT1uqd___Si : $@convention(thin) <τ_0_0><τ_1_0> (@in τ_1_0, Int) -> Int
225225
// CHECK: [[CLOSURE:%.*]] = partial_apply [[FN]]<T, ()>(%1) : $@convention(thin) <τ_0_0><τ_1_0> (@in τ_1_0, Int) -> Int
226-
// CHECK: [[THUNK:%.*]] = function_ref @_TTRGrXFo_iT__dSi_XFo__dSi_
227-
// CHECK: [[THUNK_CLOSURE:%.*]] = partial_apply [[THUNK]]<T>([[CLOSURE]])
226+
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo_iT__dSi_XFo__dSi_
227+
// CHECK: [[THUNK_CLOSURE:%.*]] = partial_apply [[THUNK]]([[CLOSURE]])
228228
// CHECK: strong_release [[THUNK_CLOSURE]]
229229
let _: () -> Int = inner_generic1
230230

‎test/SILGen/generic_objc_block_bridge.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ class Tubb<GenericParamName>: Butt {
1414
}
1515
}
1616

17-
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRGrXFdCb_dSi_dSi_XFo_dSi_dSi_ : $@convention(thin) <GenericParamName> (Int, @owned @convention(block) (Int) -> Int) -> Int {
17+
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFdCb_dSi_dSi_XFo_dSi_dSi_ : $@convention(thin) (Int, @owned @convention(block) (Int) -> Int) -> Int {

‎test/SILGen/objc_blocks_bridging.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,6 @@ struct GenericStruct<T> {
150150
}
151151
}
152152

153-
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRgrXFo_oXFo_____XFdCb_dXFdCb_____ : $@convention(c) @pseudogeneric <T> (@inout_aliasable @block_storage @callee_owned (@owned @callee_owned () -> ()) -> (), @convention(block) () -> ()) -> ()
154-
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRgrXFdCb___XFo___ : $@convention(thin) @pseudogeneric <T> (@owned @convention(block) () -> ()) -> ()
153+
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo_oXFo_____XFdCb_dXFdCb_____ : $@convention(c) (@inout_aliasable @block_storage @callee_owned (@owned @callee_owned () -> ()) -> (), @convention(block) () -> ()) -> ()
154+
// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFdCb___XFo___ : $@convention(thin) (@owned @convention(block) () -> ()) -> ()
155155

‎test/SILGen/property_abstraction.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,5 +128,5 @@ func setBuilder<F: Factory where F.Product == MyClass>(_ factory: inout F) {
128128
// CHECK: [[F1:%.*]] = thin_to_thick_function [[F0]]
129129
// CHECK: [[SETTER:%.*]] = witness_method $F, #Factory.builder!setter.1
130130
// CHECK: [[REABSTRACTOR:%.*]] = function_ref @_TTR
131-
// CHECK: [[F2:%.*]] = partial_apply [[REABSTRACTOR]]<F>([[F1]])
131+
// CHECK: [[F2:%.*]] = partial_apply [[REABSTRACTOR]]([[F1]])
132132
// CHECK: apply [[SETTER]]<F, MyClass>([[F2]], [[PB]])

0 commit comments

Comments
 (0)