Skip to content

Commit 50f6883

Browse files
committed
IRGen: Fix keypath pattern emission regression in multi-threaded mode
We were emitting relative references to entities that might be in another translation unit. Use a GOT entry or thunk where appropriate. Fixes <rdar://problem/45901706>.
1 parent 9dd63a4 commit 50f6883

File tree

5 files changed

+57
-33
lines changed

5 files changed

+57
-33
lines changed

lib/IRGen/GenDecl.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2428,9 +2428,11 @@ IRGenModule::getAddrOfLLVMVariableOrGOTEquivalent(LinkEntity entity,
24282428
// Handle SILFunctions specially, because unlike other entities they aren't
24292429
// variables and aren't kept in the GlobalVars table.
24302430
if (entity.isSILFunction()) {
2431-
auto fn = getAddrOfSILFunction(entity.getSILFunction(), NotForDefinition);
2432-
if (entity.getSILFunction()->isDefinition()
2433-
&& !isAvailableExternally(entity.getSILFunction()->getLinkage())) {
2431+
auto *silFn = entity.getSILFunction();
2432+
auto fn = getAddrOfSILFunction(silFn, NotForDefinition);
2433+
if (silFn->isDefinition() &&
2434+
!isAvailableExternally(silFn->getLinkage()) &&
2435+
this == IRGen.getGenModule(silFn)) {
24342436
return {fn, ConstantReference::Direct};
24352437
}
24362438

lib/IRGen/GenKeyPath.cpp

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ getAccessorForComputedComponent(IRGenModule &IGM,
120120
// If it's only externally available, we need a local thunk to relative-
121121
// reference.
122122
if (requirements.empty() &&
123-
!LinkEntity::forSILFunction(accessor, false).isAvailableExternally(IGM)) {
124-
123+
!isAvailableExternally(accessor->getLinkage()) &&
124+
&IGM == IGM.IRGen.getGenModule(accessor)) {
125125
return IGM.getAddrOfSILFunction(accessor, NotForDefinition);
126126
}
127127
auto accessorFn = IGM.getAddrOfSILFunction(accessor, NotForDefinition);
@@ -1030,33 +1030,23 @@ emitKeyPathComponent(IRGenModule &IGM,
10301030
fields.add(llvm::ConstantExpr::getTruncOrBitCast(idValue, IGM.Int32Ty));
10311031
break;
10321032
}
1033-
1034-
if (isInstantiableOnce) {
1035-
// No generic arguments or indexes, so we can invoke the
1036-
// getter/setter as is.
1033+
1034+
// Push the accessors, possibly thunked to marshal generic environment.
1035+
fields.addRelativeAddress(
1036+
getAccessorForComputedComponent(IGM, component, Getter,
1037+
genericEnv, requirements,
1038+
hasSubscriptIndices));
1039+
if (settable)
10371040
fields.addRelativeAddress(
1038-
IGM.getAddrOfSILFunction(component.getComputedPropertyGetter(),
1039-
NotForDefinition));
1040-
if (settable)
1041-
fields.addRelativeAddress(
1042-
IGM.getAddrOfSILFunction(component.getComputedPropertySetter(),
1043-
NotForDefinition));
1044-
} else {
1041+
getAccessorForComputedComponent(IGM, component, Setter,
1042+
genericEnv, requirements,
1043+
hasSubscriptIndices));
1044+
1045+
if (!isInstantiableOnce) {
10451046
// If there's generic context or subscript indexes, embed as
10461047
// arguments in the component. Thunk the SIL-level accessors to give the
10471048
// runtime implementation a polymorphically-callable interface.
1048-
1049-
// Push the accessors, possibly thunked to marshal generic environment.
1050-
fields.addRelativeAddress(
1051-
getAccessorForComputedComponent(IGM, component, Getter,
1052-
genericEnv, requirements,
1053-
hasSubscriptIndices));
1054-
if (settable)
1055-
fields.addRelativeAddress(
1056-
getAccessorForComputedComponent(IGM, component, Setter,
1057-
genericEnv, requirements,
1058-
hasSubscriptIndices));
1059-
1049+
10601050
fields.addRelativeAddress(
10611051
getLayoutFunctionForComputedComponent(IGM, component,
10621052
genericEnv, requirements));
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
public struct A {
2+
// note: not public
3+
var foo: Int { get { return 0 } set { } }
4+
}

test/IRGen/keypaths.sil

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,30 @@ entry:
249249
}
250250

251251
sil @k_id : $@convention(thin) () -> ()
252-
sil @k_get : $@convention(thin) (@in_guaranteed S) -> @out Int
252+
sil @k_get : $@convention(thin) (@in_guaranteed S) -> @out Int {
253+
bb0(%0 : @trivial $*Int, %1 : @trivial $*S):
254+
unreachable
255+
}
256+
257+
sil @l_get : $@convention(thin) (@in_guaranteed C) -> @out Int {
258+
bb0(%0 : @trivial $*Int, %1 : @trivial $*C):
259+
unreachable
260+
}
253261

254-
sil @l_get : $@convention(thin) (@in_guaranteed C) -> @out Int
255-
sil @l_set : $@convention(thin) (@in_guaranteed Int, @in_guaranteed C) -> ()
262+
sil @l_set : $@convention(thin) (@in_guaranteed Int, @in_guaranteed C) -> () {
263+
bb0(%0 : @trivial $*Int, %1 : @trivial $*C):
264+
unreachable
265+
}
256266

257-
sil @m_get : $@convention(thin) (@in_guaranteed S) -> @out @callee_guaranteed () -> @out ()
258-
sil @m_set : $@convention(thin) (@in_guaranteed @callee_guaranteed () -> @out (), @inout S) -> ()
267+
sil @m_get : $@convention(thin) (@in_guaranteed S) -> @out @callee_guaranteed () -> @out () {
268+
bb0(%0 : @trivial $*@callee_guaranteed () -> @out (), %1 : @trivial $*S):
269+
unreachable
270+
}
271+
272+
sil @m_set : $@convention(thin) (@in_guaranteed @callee_guaranteed () -> @out (), @inout S) -> () {
273+
bb0(%0 : @trivial $*@callee_guaranteed () -> @out (), %1 : @trivial $*S):
274+
unreachable
275+
}
259276

260277
sil @m2_get : $@convention(thin) (@in_guaranteed C2) -> @out @callee_guaranteed () -> @out ()
261278
sil @m2_set : $@convention(thin) (@in_guaranteed @callee_guaranteed () -> @out (), @inout C2) -> ()

test/IRGen/multithread_keypaths.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -c %S/Inputs/multithread_keypaths_other.swift %s -num-threads 2 -o %t/1.o -o %t/2.o -module-name multithread_keypaths
3+
// RUN: %target-swift-frontend -c %S/Inputs/multithread_keypaths_other.swift %s -num-threads 2 -o %t/1.o -o %t/2.o -module-name multithread_keypaths -enable-testing
4+
// RUN: %target-swift-frontend -c %S/Inputs/multithread_keypaths_other.swift %s -num-threads 2 -o %t/1.o -o %t/2.o -module-name multithread_keypaths -enable-resilience
5+
// RUN: %target-swift-frontend -c %S/Inputs/multithread_keypaths_other.swift %s -num-threads 2 -o %t/1.o -o %t/2.o -module-name multithread_keypaths -enable-testing -enable-resilience
6+
7+
func f(_ k: WritableKeyPath<A, Int>) {}
8+
9+
func g() {
10+
f(\A.foo)
11+
}

0 commit comments

Comments
 (0)