KeyPaths: Take a bit for encoding "let"-ness of stored properties.

If we know a key path component can be accessed as a stored property, then we should also know whether it's a `let` or not, so it should be safe to encode this in the key path pattern. Stage this change in by changing the number of bits used to store in-line offsets, fixing up the parts of the key path implementation that assumed that it took up the entire payload bitfield.

Author: jckarter

include/swift/ABI/KeyPath.h

@@ -88,55 +88,57 @@ class KeyPathComponentHeader {
   }
 
   constexpr static KeyPathComponentHeader
-  forStructComponentWithInlineOffset(unsigned offset) {
+  forStructComponentWithInlineOffset(bool isLet,
+                                     unsigned offset) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_StructTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)
       | validateInlineOffset(offset));
   }
 
   constexpr static KeyPathComponentHeader
-  forStructComponentWithOutOfLineOffset() {
+  forStructComponentWithOutOfLineOffset(bool isLet) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_StructTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)
       | _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload);
   }
 
   constexpr static KeyPathComponentHeader
-  forStructComponentWithUnresolvedFieldOffset() {
+  forStructComponentWithUnresolvedFieldOffset(bool isLet) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_StructTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)
       | _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload);
   }
 
   constexpr static KeyPathComponentHeader
-  forClassComponentWithInlineOffset(unsigned offset) {
+  forClassComponentWithInlineOffset(bool isLet,
+                                    unsigned offset) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_ClassTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)
       | validateInlineOffset(offset));
   }
 
   constexpr static KeyPathComponentHeader
-  forClassComponentWithOutOfLineOffset() {
+  forClassComponentWithOutOfLineOffset(bool isLet) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_ClassTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)
       | _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload);
   }
 
   constexpr static KeyPathComponentHeader
-  forClassComponentWithUnresolvedFieldOffset() {
+  forClassComponentWithUnresolvedFieldOffset(bool isLet) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_ClassTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)
       | _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload);
   }
 
   constexpr static KeyPathComponentHeader
-  forClassComponentWithUnresolvedIndirectOffset() {
+  forClassComponentWithUnresolvedIndirectOffset(bool isLet) {
     return KeyPathComponentHeader(
       (_SwiftKeyPathComponentHeader_ClassTag
       << _SwiftKeyPathComponentHeader_DiscriminatorShift)

lib/IRGen/GenKeyPath.cpp

@@ -773,22 +773,23 @@ emitKeyPathComponent(IRGenModule &IGM,
   case KeyPathPatternComponent::Kind::StoredProperty: {
     auto property = cast<VarDecl>(component.getStoredPropertyDecl());
 
-    auto addFixedOffset = [&](bool isStruct, llvm::Constant *offset) {
+    auto addFixedOffset = [&](bool isStruct, bool isLet,
+                              llvm::Constant *offset) {
       if (auto offsetInt = dyn_cast_or_null<llvm::ConstantInt>(offset)) {
         auto offsetValue = offsetInt->getValue().getZExtValue();
         if (KeyPathComponentHeader::offsetCanBeInline(offsetValue)) {
           auto header = isStruct
             ? KeyPathComponentHeader
-                ::forStructComponentWithInlineOffset(offsetValue)
+                ::forStructComponentWithInlineOffset(isLet, offsetValue)
             : KeyPathComponentHeader
-                ::forClassComponentWithInlineOffset(offsetValue);
+                ::forClassComponentWithInlineOffset(isLet, offsetValue);
           fields.addInt32(header.getData());
           return;
         }
       }
       auto header = isStruct
-        ? KeyPathComponentHeader::forStructComponentWithOutOfLineOffset()
-        : KeyPathComponentHeader::forClassComponentWithOutOfLineOffset();
+        ? KeyPathComponentHeader::forStructComponentWithOutOfLineOffset(isLet)
+        : KeyPathComponentHeader::forClassComponentWithOutOfLineOffset(isLet);
       fields.addInt32(header.getData());
       fields.add(llvm::ConstantExpr::getTruncOrBitCast(offset, IGM.Int32Ty));
     };
@@ -801,7 +802,7 @@ emitKeyPathComponent(IRGenModule &IGM,
                                                             loweredBaseTy,
                                                             property)) {
         // We have a known constant fixed offset.
-        addFixedOffset(/*struct*/ true, offset);
+        addFixedOffset(/*struct*/ true, property->isLet(), offset);
         break;
       }
 
@@ -811,7 +812,7 @@ emitKeyPathComponent(IRGenModule &IGM,
       auto fieldOffset = metadataLayout.getStaticFieldOffset(property);
 
       auto header = KeyPathComponentHeader
-        ::forStructComponentWithUnresolvedFieldOffset();
+        ::forStructComponentWithUnresolvedFieldOffset(property->isLet());
       fields.addInt32(header.getData());
       fields.addInt32(fieldOffset.getValue());
       break;
@@ -829,14 +830,14 @@ emitKeyPathComponent(IRGenModule &IGM,
                                                                 loweredBaseTy,
                                                                 property);
         assert(offset && "no constant offset for ConstantDirect field?!");
-        addFixedOffset(/*struct*/ false, offset);
+        addFixedOffset(/*struct*/ false, property->isLet(), offset);
         break;
       }
       case FieldAccess::NonConstantDirect: {
         // A constant offset that's determined at class realization time.
         // We have to load the offset from a global ivar.
         auto header = KeyPathComponentHeader
-          ::forClassComponentWithUnresolvedIndirectOffset();
+          ::forClassComponentWithUnresolvedIndirectOffset(property->isLet());
         fields.addInt32(header.getData());
         fields.addAlignmentPadding(IGM.getPointerAlignment());
         auto offsetVar = IGM.getAddrOfFieldOffset(property, NotForDefinition);
@@ -846,8 +847,8 @@ emitKeyPathComponent(IRGenModule &IGM,
       case FieldAccess::ConstantIndirect: {
         // An offset that depends on the instance's generic parameterization,
         // but whose field offset is at a known vtable offset.
-        auto header =
-          KeyPathComponentHeader::forClassComponentWithUnresolvedFieldOffset();
+        auto header = KeyPathComponentHeader
+          ::forClassComponentWithUnresolvedFieldOffset(property->isLet());
         fields.addInt32(header.getData());
         auto fieldOffset =
           getClassFieldOffsetOffset(IGM,

stdlib/public/SwiftShims/KeyPath.h

@@ -59,17 +59,23 @@ static const __swift_uint32_t _SwiftKeyPathComponentHeader_ExternalTag
 static const __swift_uint32_t
 _SwiftKeyPathComponentHeader_TrivialPropertyDescriptorMarker = 0U;
 
+static const __swift_uint32_t _SwiftKeyPathComponentHeader_StoredOffsetPayloadMask
+  = 0x007FFFFFU;
+
 static const __swift_uint32_t _SwiftKeyPathComponentHeader_MaximumOffsetPayload
-  = 0x00FFFFFCU;
+  = 0x007FFFFCU;
 
 static const __swift_uint32_t _SwiftKeyPathComponentHeader_UnresolvedIndirectOffsetPayload
-  = 0x00FFFFFDU;
+  = 0x007FFFFDU;
 
 static const __swift_uint32_t _SwiftKeyPathComponentHeader_UnresolvedFieldOffsetPayload
-  = 0x00FFFFFEU;
+  = 0x007FFFFEU;
 
 static const __swift_uint32_t _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload
-  = 0x00FFFFFFU;
+  = 0x007FFFFFU;
+  
+static const __swift_uint32_t _SwiftKeyPathComponentHeader_StoredMutableFlag
+  = 0x00800000U;
 
 static const __swift_uint32_t _SwiftKeyPathComponentHeader_OptionalChainPayload
   = 0;

stdlib/public/core/KeyPath.swift

@@ -758,6 +758,9 @@ internal struct RawKeyPathComponent {
     internal static var endOfReferencePrefixFlag: UInt32 {
       return _SwiftKeyPathComponentHeader_EndOfReferencePrefixFlag
     }
+    internal static var storedOffsetPayloadMask: UInt32 {
+      return _SwiftKeyPathComponentHeader_StoredOffsetPayloadMask
+    }
     internal static var outOfLineOffsetPayload: UInt32 {
       return _SwiftKeyPathComponentHeader_OutOfLineOffsetPayload
     }
@@ -852,6 +855,20 @@ internal struct RawKeyPathComponent {
         _value = _value & ~Header.payloadMask | newValue
       }
     }
+    internal var storedOffsetPayload: UInt32 {
+      get {
+        _sanityCheck(kind == .struct || kind == .class,
+                     "not a stored component")
+        return _value & Header.storedOffsetPayloadMask
+      }
+      set {
+        _sanityCheck(kind == .struct || kind == .class,
+                     "not a stored component")
+        _sanityCheck(newValue & Header.storedOffsetPayloadMask == newValue,
+                     "payload too big")
+        _value = _value & ~Header.storedOffsetPayloadMask | newValue
+      }
+    }
     internal var endOfReferencePrefix: Bool {
       get {
         return _value & Header.endOfReferencePrefixFlag != 0
@@ -913,12 +930,12 @@ internal struct RawKeyPathComponent {
     internal func _componentBodySize(forPropertyDescriptor: Bool) -> Int {
       switch kind {
       case .struct, .class:
-        if payload == Header.unresolvedFieldOffsetPayload
-           || payload == Header.outOfLineOffsetPayload {
+        if storedOffsetPayload == Header.unresolvedFieldOffsetPayload
+           || storedOffsetPayload == Header.outOfLineOffsetPayload {
           // A 32-bit offset is stored in the body.
           return MemoryLayout<UInt32>.size
         }
-        if payload == Header.unresolvedIndirectOffsetPayload {
+        if storedOffsetPayload == Header.unresolvedIndirectOffsetPayload {
           // A pointer-aligned, pointer-sized pointer is stored in the body.
           return Header.pointerAlignmentSkew + MemoryLayout<Int>.size
         }
@@ -959,7 +976,9 @@ internal struct RawKeyPathComponent {
     let ptrSize = MemoryLayout<Int>.size
     switch header.kind {
     case .struct, .class:
-      if header.payload == Header.payloadMask { return 4 } // overflowed
+      if header.storedOffsetPayload == Header.outOfLineOffsetPayload {
+        return 4 // overflowed
+      }
       return 0
     case .external:
       // align to pointer + pointer to external descriptor
@@ -993,13 +1012,13 @@ internal struct RawKeyPathComponent {
                  "no offset for this kind")
     // An offset too large to fit inline is represented by a signal and stored
     // in the body.
-    if header.payload == Header.outOfLineOffsetPayload {
+    if header.storedOffsetPayload == Header.outOfLineOffsetPayload {
       // Offset overflowed into body
       _sanityCheck(body.count >= MemoryLayout<UInt32>.size,
                    "component not big enough")
       return Int(body.load(as: UInt32.self))
     }
-    return Int(header.payload)
+    return Int(header.storedOffsetPayload)
   }
 
   internal var _computedIDValue: Int {
@@ -1165,7 +1184,7 @@ internal struct RawKeyPathComponent {
     switch header.kind {
     case .struct,
          .class:
-      if header.payload == Header.outOfLineOffsetPayload {
+      if header.storedOffsetPayload == Header.outOfLineOffsetPayload {
         let overflowOffset = body.load(as: UInt32.self)
         buffer.storeBytes(of: overflowOffset, toByteOffset: 4,
                           as: UInt32.self)
@@ -2292,11 +2311,11 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern(
       // reassigned.
 
       // Check the final instantiated size of the offset.
-      if header.payload == RawKeyPathComponent.Header.unresolvedFieldOffsetPayload
-        || header.payload == RawKeyPathComponent.Header.outOfLineOffsetPayload {
+      if header.storedOffsetPayload == RawKeyPathComponent.Header.unresolvedFieldOffsetPayload
+        || header.storedOffsetPayload == RawKeyPathComponent.Header.outOfLineOffsetPayload {
         _ = buffer.pop(UInt32.self)
       }
-      if header.payload == RawKeyPathComponent.Header.unresolvedIndirectOffsetPayload {
+      if header.storedOffsetPayload == RawKeyPathComponent.Header.unresolvedIndirectOffsetPayload {
         _ = buffer.pop(Int.self)
         // On 64-bit systems the pointer to the ivar offset variable is
         // pointer-sized and -aligned, but the resulting offset ought to be
@@ -2363,7 +2382,7 @@ internal func _getKeyPathClassAndInstanceSizeFromPattern(
         // The final component will be a stored component with just an offset.
         // If the offset requires resolution, then it'll be stored out of
         // line after the header.
-        if descriptorHeader.payload
+        if descriptorHeader.storedOffsetPayload
             > RawKeyPathComponent.Header.maximumOffsetPayload {
           newComponentSize = MemoryLayout<RawKeyPathComponent.Header>.size
                            + MemoryLayout<UInt32>.size
@@ -2617,7 +2636,7 @@ internal func _instantiateKeyPathBuffer(
 
     func tryToResolveOffset(header: RawKeyPathComponent.Header,
                             getOutOfLineOffset: () -> UInt32) {
-      if header.payload == RawKeyPathComponent.Header.unresolvedFieldOffsetPayload {
+      if header.storedOffsetPayload == RawKeyPathComponent.Header.unresolvedFieldOffsetPayload {
         // Look up offset in type metadata. The value in the pattern is the
         // offset within the metadata object.
         let metadataPtr = unsafeBitCast(base, to: UnsafeRawPointer.self)
@@ -2634,28 +2653,28 @@ internal func _instantiateKeyPathBuffer(
 
         // Rewrite the header for a resolved offset.
         var newHeader = header
-        newHeader.payload = RawKeyPathComponent.Header.outOfLineOffsetPayload
+        newHeader.storedOffsetPayload = RawKeyPathComponent.Header.outOfLineOffsetPayload
         pushDest(newHeader)
         pushDest(offset)
         return
       }
 
-      if header.payload == RawKeyPathComponent.Header.unresolvedIndirectOffsetPayload {
+      if header.storedOffsetPayload == RawKeyPathComponent.Header.unresolvedIndirectOffsetPayload {
         // Look up offset in the indirectly-referenced variable we have a
         // pointer.
         let offsetVar = patternBuffer.pop(UnsafeRawPointer.self)
         let offsetValue = UInt32(offsetVar.load(as: UInt.self))
         // Rewrite the header for a resolved offset.
         var newHeader = header
-        newHeader.payload = RawKeyPathComponent.Header.outOfLineOffsetPayload
+        newHeader.storedOffsetPayload = RawKeyPathComponent.Header.outOfLineOffsetPayload
         pushDest(newHeader)
         pushDest(offsetValue)
         return
       }
 
       // Otherwise, just transfer the pre-resolved component.
       pushDest(header)
-      if header.payload == RawKeyPathComponent.Header.outOfLineOffsetPayload {
+      if header.storedOffsetPayload == RawKeyPathComponent.Header.outOfLineOffsetPayload {
         let offset = getOutOfLineOffset() //patternBuffer.pop(UInt32.self)
         pushDest(offset)
       }

test/IRGen/keypaths.sil

@@ -205,7 +205,7 @@ sil_vtable C {}
 // CHECK-SAME: i32 8,
 // CHECK-64-SAME: [4 x i8] zeroinitializer,
 //             -- struct with runtime-resolved offset
-// CHECK-SAME: <i32 0x01fffffe>,
+// CHECK-SAME: <i32 0x017ffffe>,
 // CHECK-32-SAME: i32 16 }>
 // CHECK-64-SAME: i32 32 }>
 
@@ -218,7 +218,7 @@ sil_vtable C {}
 // CHECK-SAME: i32 8,
 // CHECK-64-SAME: [4 x i8] zeroinitializer,
 //             -- struct with runtime-resolved offset
-// CHECK-SAME: <i32 0x01fffffe>,
+// CHECK-SAME: <i32 0x017ffffe>,
 // CHECK-32-SAME: i32 20 }>
 // CHECK-64-SAME: i32 36 }>
 

test/IRGen/keypaths_objc.sil

@@ -23,8 +23,8 @@ sil @x_get : $@convention(thin) (@in_guaranteed C) -> @out NSString
 // CHECK-SAME: i8** @"\01L_selector(x)"
 
 // CHECK: [[KEYPATH_B:@keypath.*]] = private global
-// --             0x03fffffd: class stored property with indirect offset
-// CHECK-SAME: <i32 0x03fffffd>,
+// --             0x037ffffd: class stored property with indirect offset
+// CHECK-SAME: <i32 0x037ffffd>,
 // CHECK-SAME: @"$S13keypaths_objc1CC6storedSivpWvd"
 
 // CHECK-LABEL: define swiftcc void @objc_only_property()

test/IRGen/property_descriptor.sil

@@ -39,17 +39,15 @@ public struct ExternalReabstractions<T> {
   @sil_stored public var reabstracted: () -> () { get set }
 }
 
-// -- 0xff_fffe - struct property, offset resolved from field offset vector in metadata
+// -- struct property, offset resolved from field offset vector in metadata
 // CHECK-64: @"$S19property_descriptor15ExternalGenericV2roxvpMV" =
-// CHECK-64-SAME: <{ <i32 0x01ff_fffe>, i32 32 }>, align 8
 // CHECK-32: @"$S19property_descriptor15ExternalGenericV2roxvpMV" =
-// CHECK-32-SAME: <{ <i32 0x01ff_fffe>, i32 16 }>, align 4
+// CHECK-SAME: <{ <i32 0x017f_fffe>,
 sil_property #ExternalGeneric.ro <T: Comparable> (
   stored_property #ExternalGeneric.ro : $T)
 // CHECK-64: @"$S19property_descriptor15ExternalGenericV2rwxvpMV" =
-// CHECK-64-SAME: <{ <i32 0x01ff_fffe>, i32 36 }>, align 8
 // CHECK-32: @"$S19property_descriptor15ExternalGenericV2rwxvpMV" =
-// CHECK-32-SAME: <{ <i32 0x01ff_fffe>, i32 20 }>, align 4
+// CHECK-SAME: <{ <i32 0x017f_fffe>,
 sil_property #ExternalGeneric.rw <T: Comparable> (
   stored_property #ExternalGeneric.rw : $T)