Skip to content

Commit de6318d

Browse files
jckarterjckarter
authored
Merge pull request #21774 from jckarter/keypath-selector-instantiation-fn
KeyPaths: Don't relative-reference selector refs.
2 parents 6472474 + 9669c3a commit de6318d

File tree

7 files changed

+115
-35
lines changed

7 files changed

+115
-35
lines changed

‎include/swift/ABI/KeyPath.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,17 +190,17 @@ class KeyPathComponentHeader {
190190
VTableOffset,
191191
};
192192

193-
constexpr static uint32_t
194-
getResolutionStrategy(ComputedPropertyIDKind idKind) {
195-
return idKind == Pointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
196-
: (assert("no resolution strategy implemented" && false), 0);
197-
}
193+
enum ComputedPropertyIDResolution {
194+
Resolved,
195+
IndirectPointer,
196+
FunctionCall,
197+
};
198198

199199
constexpr static KeyPathComponentHeader
200200
forComputedProperty(ComputedPropertyKind kind,
201201
ComputedPropertyIDKind idKind,
202202
bool hasArguments,
203-
bool resolvedID) {
203+
ComputedPropertyIDResolution resolution) {
204204
return KeyPathComponentHeader(
205205
(_SwiftKeyPathComponentHeader_ComputedTag
206206
<< _SwiftKeyPathComponentHeader_DiscriminatorShift)
@@ -213,8 +213,10 @@ class KeyPathComponentHeader {
213213
| (idKind == VTableOffset
214214
? _SwiftKeyPathComponentHeader_ComputedIDByVTableOffsetFlag : 0)
215215
| (hasArguments ? _SwiftKeyPathComponentHeader_ComputedHasArgumentsFlag : 0)
216-
| (resolvedID ? _SwiftKeyPathComponentHeader_ComputedIDResolved
217-
: getResolutionStrategy(idKind)));
216+
| (resolution == Resolved ? _SwiftKeyPathComponentHeader_ComputedIDResolved
217+
: resolution == IndirectPointer ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
218+
: resolution == FunctionCall ? _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
219+
: (assert(false && "invalid resolution"), 0)));
218220
}
219221

220222
constexpr static KeyPathComponentHeader

‎lib/IRGen/GenKeyPath.cpp

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "TypeInfo.h"
3737
#include "llvm/ADT/SetVector.h"
3838
#include "llvm/IR/Module.h"
39+
#include "llvm/IR/Function.h"
3940
#include "swift/SIL/SILInstruction.h"
4041
#include "swift/SIL/SILLocation.h"
4142
#include "swift/SIL/TypeLowering.h"
@@ -947,7 +948,7 @@ emitKeyPathComponent(IRGenModule &IGM,
947948
auto id = component.getComputedPropertyId();
948949
KeyPathComponentHeader::ComputedPropertyIDKind idKind;
949950
llvm::Constant *idValue;
950-
bool idResolved;
951+
KeyPathComponentHeader::ComputedPropertyIDResolution idResolution;
951952
switch (id.getKind()) {
952953
case KeyPathPatternComponent::ComputedPropertyId::Function: {
953954
idKind = KeyPathComponentHeader::Pointer;
@@ -957,7 +958,9 @@ emitKeyPathComponent(IRGenModule &IGM,
957958
idValue = idRef.getValue();
958959
// If we got an indirect reference, we'll need to resolve it at
959960
// instantiation time.
960-
idResolved = !idRef.isIndirect();
961+
idResolution = idRef.isIndirect()
962+
? KeyPathComponentHeader::IndirectPointer
963+
: KeyPathComponentHeader::Resolved;
961964
break;
962965
}
963966
case KeyPathPatternComponent::ComputedPropertyId::DeclRef: {
@@ -969,8 +972,35 @@ emitKeyPathComponent(IRGenModule &IGM,
969972
if (declRef.isForeign) {
970973
assert(IGM.ObjCInterop && "foreign keypath component w/o objc interop?!");
971974
idKind = KeyPathComponentHeader::Pointer;
972-
idValue = IGM.getAddrOfObjCSelectorRef(declRef);
973-
idResolved = false;
975+
// FIXME: In non-JIT mode, ideally we would just refer to the selector
976+
// reference variable here with an indirectpointer resolution,
977+
// but ld64 section coalescing on the __objc_sel section can break
978+
// relative references (and on some platforms, mach-o just doesn't
979+
// support the necessary relocations).
980+
// As a workaround, generate a stub function to resolve the selector.
981+
//
982+
// Note that we'd need to do this anyway in JIT mode because we would
983+
// need to unique the selector at runtime anyway.
984+
auto selectorName = IGM.getObjCSelectorName(declRef);
985+
llvm::Type *fnParams[] = {IGM.Int8PtrTy};
986+
auto fnTy = llvm::FunctionType::get(IGM.Int8PtrTy, fnParams, false);
987+
SmallString<32> fnName;
988+
fnName.append("keypath_get_selector_");
989+
fnName.append(selectorName);
990+
auto fn = cast<llvm::Function>(
991+
IGM.Module.getOrInsertFunction(fnName, fnTy));
992+
if (fn->empty()) {
993+
fn->setLinkage(llvm::Function::PrivateLinkage);
994+
IRGenFunction subIGF(IGM, fn);
995+
if (IGM.DebugInfo)
996+
IGM.DebugInfo->emitArtificialFunction(subIGF, fn);
997+
998+
auto selectorValue = subIGF.emitObjCSelectorRefLoad(selectorName);
999+
subIGF.Builder.CreateRet(selectorValue);
1000+
}
1001+
1002+
idValue = fn;
1003+
idResolution = KeyPathComponentHeader::FunctionCall;
9741004
} else {
9751005
if (auto overridden = declRef.getOverriddenVTableEntry())
9761006
declRef = overridden;
@@ -989,7 +1019,9 @@ emitKeyPathComponent(IRGenModule &IGM,
9891019
LinkEntity::forMethodDescriptor(declRef));
9901020

9911021
idValue = idRef.getValue();
992-
idResolved = !idRef.isIndirect();
1022+
idResolution = idRef.isIndirect()
1023+
? KeyPathComponentHeader::IndirectPointer
1024+
: KeyPathComponentHeader::Resolved;
9931025
break;
9941026
}
9951027

@@ -1000,7 +1032,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10001032
auto index = protoInfo.getFunctionIndex(
10011033
cast<AbstractFunctionDecl>(declRef.getDecl()));
10021034
idValue = llvm::ConstantInt::get(IGM.SizeTy, -index.getValue());
1003-
idResolved = true;
1035+
idResolution = KeyPathComponentHeader::Resolved;
10041036
}
10051037
break;
10061038
}
@@ -1013,7 +1045,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10131045
// Scan the stored properties of the struct to find the index. We should
10141046
// only ever use a struct field as a uniquing key from inside the
10151047
// struct's own module, so this is OK.
1016-
idResolved = true;
1048+
idResolution = KeyPathComponentHeader::Resolved;
10171049
Optional<unsigned> structIdx;
10181050
unsigned i = 0;
10191051
for (auto storedProp : struc->getStoredProperties()) {
@@ -1034,7 +1066,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10341066
case FieldAccess::ConstantDirect:
10351067
case FieldAccess::ConstantIndirect:
10361068
case FieldAccess::NonConstantDirect:
1037-
idResolved = true;
1069+
idResolution = KeyPathComponentHeader::Resolved;
10381070
idValue = llvm::ConstantInt::get(IGM.SizeTy,
10391071
getClassFieldIndex(classDecl, property));
10401072
break;
@@ -1047,7 +1079,7 @@ emitKeyPathComponent(IRGenModule &IGM,
10471079
}
10481080

10491081
auto header = KeyPathComponentHeader::forComputedProperty(componentKind,
1050-
idKind, !isInstantiableOnce, idResolved);
1082+
idKind, !isInstantiableOnce, idResolution);
10511083

10521084
fields.addInt32(header.getData());
10531085
switch (idKind) {

‎lib/IRGen/GenObjC.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,11 @@ llvm::Constant *IRGenModule::getAddrOfObjCSelectorRef(SILDeclRef method) {
534534
return getAddrOfObjCSelectorRef(Selector(method).str());
535535
}
536536

537+
std::string IRGenModule::getObjCSelectorName(SILDeclRef method) {
538+
assert(method.isForeign);
539+
return Selector(method).str();
540+
}
541+
537542
static llvm::Value *emitSuperArgument(IRGenFunction &IGF,
538543
bool isInstanceMethod,
539544
llvm::Value *selfValue,

‎lib/IRGen/IRGenModule.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,7 @@ class IRGenModule {
828828
llvm::Constant *getAddrOfGlobalUTF16String(StringRef utf8);
829829
llvm::Constant *getAddrOfObjCSelectorRef(StringRef selector);
830830
llvm::Constant *getAddrOfObjCSelectorRef(SILDeclRef method);
831+
std::string getObjCSelectorName(SILDeclRef method);
831832
llvm::Constant *getAddrOfObjCMethodName(StringRef methodName);
832833
llvm::Constant *getAddrOfObjCProtocolRecord(ProtocolDecl *proto,
833834
ForDefinition_t forDefinition);

‎stdlib/public/SwiftShims/KeyPath.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDResolved
106106
= 0x00000000U;
107107
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
108108
= 0x00000002U;
109+
static const __swift_uint32_t _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
110+
= 0x00000001U;
109111

110112
extern const void *_Nonnull (swift_keyPathGenericWitnessTable[]);
111113

‎stdlib/public/core/KeyPath.swift

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,12 @@ internal enum KeyPathComputedIDKind {
739739
case vtableOffset
740740
}
741741

742+
internal enum KeyPathComputedIDResolution {
743+
case resolved
744+
case indirectPointer
745+
case functionCall
746+
}
747+
742748
internal struct RawKeyPathComponent {
743749
internal init(header: Header, body: UnsafeRawBufferPointer) {
744750
self.header = header
@@ -890,9 +896,20 @@ internal struct RawKeyPathComponent {
890896
internal static var computedIDUnresolvedIndirectPointer: UInt32 {
891897
return _SwiftKeyPathComponentHeader_ComputedIDUnresolvedIndirectPointer
892898
}
893-
internal var isComputedIDResolved: Bool {
894-
return
895-
payload & Header.computedIDResolutionMask == Header.computedIDResolved
899+
internal static var computedIDUnresolvedFunctionCall: UInt32 {
900+
return _SwiftKeyPathComponentHeader_ComputedIDUnresolvedFunctionCall
901+
}
902+
internal var computedIDResolution: KeyPathComputedIDResolution {
903+
switch payload & Header.computedIDResolutionMask {
904+
case Header.computedIDResolved:
905+
return .resolved
906+
case Header.computedIDUnresolvedIndirectPointer:
907+
return .indirectPointer
908+
case Header.computedIDUnresolvedFunctionCall:
909+
return .functionCall
910+
default:
911+
_internalInvariantFailure("invalid key path resolution")
912+
}
896913
}
897914

898915
internal var _value: UInt32
@@ -2512,7 +2529,7 @@ internal protocol KeyPathPatternVisitor {
25122529
offset: KeyPathPatternStoredOffset)
25132530
mutating func visitComputedComponent(mutating: Bool,
25142531
idKind: KeyPathComputedIDKind,
2515-
idResolved: Bool,
2532+
idResolution: KeyPathComputedIDResolution,
25162533
idValueBase: UnsafeRawPointer,
25172534
idValue: Int32,
25182535
getter: UnsafeRawPointer,
@@ -2694,7 +2711,7 @@ internal func _walkKeyPathPattern<W: KeyPathPatternVisitor>(
26942711

26952712
walker.visitComputedComponent(mutating: header.isComputedMutating,
26962713
idKind: header.computedIDKind,
2697-
idResolved: header.isComputedIDResolved,
2714+
idResolution: header.computedIDResolution,
26982715
idValueBase: idValueBase,
26992716
idValue: idValue,
27002717
getter: getter,
@@ -2785,7 +2802,7 @@ internal func _walkKeyPathPattern<W: KeyPathPatternVisitor>(
27852802
walker.visitComputedComponent(
27862803
mutating: descriptorHeader.isComputedMutating,
27872804
idKind: descriptorHeader.computedIDKind,
2788-
idResolved: descriptorHeader.isComputedIDResolved,
2805+
idResolution: descriptorHeader.computedIDResolution,
27892806
idValueBase: idValueBase,
27902807
idValue: idValue,
27912808
getter: getter,
@@ -2893,7 +2910,7 @@ internal struct GetKeyPathClassAndInstanceSizeFromPattern
28932910

28942911
mutating func visitComputedComponent(mutating: Bool,
28952912
idKind: KeyPathComputedIDKind,
2896-
idResolved: Bool,
2913+
idResolution: KeyPathComputedIDResolution,
28972914
idValueBase: UnsafeRawPointer,
28982915
idValue: Int32,
28992916
getter: UnsafeRawPointer,
@@ -3149,7 +3166,7 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
31493166

31503167
mutating func visitComputedComponent(mutating: Bool,
31513168
idKind: KeyPathComputedIDKind,
3152-
idResolved: Bool,
3169+
idResolution: KeyPathComputedIDResolution,
31533170
idValueBase: UnsafeRawPointer,
31543171
idValue: Int32,
31553172
getter: UnsafeRawPointer,
@@ -3168,7 +3185,7 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
31683185

31693186
switch idKind {
31703187
case .storedPropertyIndex, .vtableOffset:
3171-
_internalInvariant(idResolved)
3188+
_internalInvariant(idResolution == .resolved)
31723189
// Zero-extend the integer value to get the instantiated id.
31733190
let value = UInt(UInt32(bitPattern: idValue))
31743191
resolvedID = UnsafeRawPointer(bitPattern: value)
@@ -3177,11 +3194,26 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
31773194
// Resolve the sign-extended relative reference.
31783195
var absoluteID: UnsafeRawPointer? = idValueBase + Int(idValue)
31793196

3180-
// If the pointer ID is "unresolved", then it needs another indirection
3181-
// to get the final value.
3182-
if !idResolved {
3197+
// If the pointer ID is unresolved, then it needs work to get to
3198+
// the final value.
3199+
switch idResolution {
3200+
case .resolved:
3201+
break
3202+
3203+
case .indirectPointer:
3204+
// The pointer in the pattern is an indirect pointer to the real
3205+
// identifier pointer.
31833206
absoluteID = absoluteID.unsafelyUnwrapped
31843207
.load(as: UnsafeRawPointer?.self)
3208+
3209+
case .functionCall:
3210+
// The pointer in the pattern is to a function that generates the
3211+
// identifier pointer.
3212+
typealias Resolver = @convention(c) (UnsafeRawPointer?) -> UnsafeRawPointer?
3213+
let resolverFn = unsafeBitCast(absoluteID.unsafelyUnwrapped,
3214+
to: Resolver.self)
3215+
3216+
absoluteID = resolverFn(patternArgs)
31853217
}
31863218
resolvedID = absoluteID
31873219
}
@@ -3341,7 +3373,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
33413373
}
33423374
mutating func visitComputedComponent(mutating: Bool,
33433375
idKind: KeyPathComputedIDKind,
3344-
idResolved: Bool,
3376+
idResolution: KeyPathComputedIDResolution,
33453377
idValueBase: UnsafeRawPointer,
33463378
idValue: Int32,
33473379
getter: UnsafeRawPointer,
@@ -3350,7 +3382,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
33503382
externalArgs: UnsafeBufferPointer<Int32>?) {
33513383
sizeVisitor.visitComputedComponent(mutating: mutating,
33523384
idKind: idKind,
3353-
idResolved: idResolved,
3385+
idResolution: idResolution,
33543386
idValueBase: idValueBase,
33553387
idValue: idValue,
33563388
getter: getter,
@@ -3359,7 +3391,7 @@ internal struct ValidatingInstantiateKeyPathBuffer: KeyPathPatternVisitor {
33593391
externalArgs: externalArgs)
33603392
instantiateVisitor.visitComputedComponent(mutating: mutating,
33613393
idKind: idKind,
3362-
idResolved: idResolved,
3394+
idResolution: idResolution,
33633395
idValueBase: idValueBase,
33643396
idValue: idValue,
33653397
getter: getter,

‎test/IRGen/keypaths_objc.sil

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ sil_vtable C {}
1818
sil @x_get : $@convention(thin) (@in_guaranteed C) -> @out NSString
1919

2020
// CHECK: [[KEYPATH_A:@keypath(\..*)?]] = private global
21-
// -- computed, get-only, indirect identifier
22-
// CHECK-SAME: <i32 0x0200_0002>,
23-
// CHECK-SAME: i8** @"\01L_selector(x)"
21+
// -- computed, get-only, function-instantiated identifier
22+
// CHECK-SAME: <i32 0x0200_0001>,
23+
// CHECK-SAME: i8* (i8*)* [[SELECTOR_FN:@[A-Za-z0-9_.]+]] to
2424

2525
// CHECK: [[KEYPATH_B:@keypath(\..*)?]] = private global
2626
// -- class mutable stored property with indirect offset
@@ -45,6 +45,12 @@ entry(%0 : $@objc_metatype C.Type):
4545
unreachable
4646
}
4747

48+
// CHECK: define private i8* [[SELECTOR_FN]]
49+
// CHECK-NEXT: entry:
50+
// CHECK-NEXT: %1 = load {{.*}}selector(x)
51+
// CHECK-NEXT: ret i8* %1
52+
53+
// CHECK-LABEL: define swiftcc void @objc_stored_property()
4854
sil @objc_stored_property : $@convention(thin) () -> () {
4955
entry:
5056
// CHECK: call %swift.refcounted* @swift_getKeyPath({{.*}} [[KEYPATH_B]]

0 commit comments

Comments
 (0)