Merge pull request #75429 from tbkka/tbkka-mpe-improvements-2024-07-23

More MPE improvements

stdlib/public/RemoteInspection/TypeLowering.cpp

@@ -327,9 +327,7 @@ BitMask BuiltinTypeInfo::getSpareBits(TypeConverter &TC, bool &hasAddrOnly) cons
     auto mask = BitMask::oneMask(getSize());
     mask.keepOnlyMostSignificantBits(getSize() * 8 - intSize);
     return mask;
-  } else if (
-    Name == "ypXp" || // Any.Type
-    Name == "yyXf" // 'yyXf' =  @thin () -> Void function
+  } else if (Name == "ypXp" // Any.Type
   ) {
     // Builtin types that expose pointer spare bits
     auto mpePointerSpareBits = TC.getBuilder().getMultiPayloadEnumPointerMask();
@@ -413,15 +411,22 @@ bool RecordTypeInfo::readExtraInhabitantIndex(remote::MemoryReader &reader,
 }
 
 BitMask RecordTypeInfo::getSpareBits(TypeConverter &TC, bool &hasAddrOnly) const {
+  // Start with all spare bits; we'll mask them out as we go...
   auto mask = BitMask::oneMask(getSize());
   switch (SubKind) {
   case RecordKind::Invalid:
-    return mask;    // FIXME: Should invalid have all spare bits?  Or none?  Does it matter?
+    // FIXME: Should invalid have all spare bits?  Or none?  Does it matter?
+    return mask;
   case RecordKind::Tuple:
   case RecordKind::Struct:
+    // Regular aggregates inherit spare bits from their fields
     break;
   case RecordKind::ThickFunction:
-    break;
+    // Thick functions have two fields:
+    // * Code pointer that might be signed and/or misaligned
+    // * Context that could be a tagged pointer
+    mask.makeZero(); // No spare bits
+    return mask;
   case RecordKind::OpaqueExistential: {
     // Existential storage isn't recorded as a field,
     // so we handle it specially here...
@@ -429,12 +434,19 @@ BitMask RecordTypeInfo::getSpareBits(TypeConverter &TC, bool &hasAddrOnly) const
     BitMask submask = BitMask::zeroMask(pointerSize * 3);
     mask.andMask(submask, 0);
     hasAddrOnly = true;
+    // Mask the rest of the fields as usual...
     break;
   }
   case RecordKind::ClassExistential:
-    break;
-  case RecordKind::ExistentialMetatype:
-    break; // Field 0 is metadata pointer, a Builtin of type 'yyXf'
+    // Class existential is a data pointer that does expose spare bits
+    // ... so we can fall through ...
+  case RecordKind::ExistentialMetatype: {
+    // Initial metadata pointer has spare bits
+    auto mpePointerSpareBits = TC.getBuilder().getMultiPayloadEnumPointerMask();
+    mask.andMask(mpePointerSpareBits, 0);
+    mask.keepOnlyLeastSignificantBytes(TC.targetPointerSize());
+    return mask;
+  }
   case RecordKind::ErrorExistential:
     break;
   case RecordKind::ClassInstance:
@@ -1071,7 +1083,20 @@ public:
   BitMask getMultiPayloadTagBitsMask() const {
     auto payloadTagValues = NumEffectivePayloadCases - 1;
     if (getNumCases() > NumEffectivePayloadCases) {
-      payloadTagValues += 1;
+      // How many payload bits are there?
+      auto payloadBits = spareBitsMask;
+      payloadBits.complement(); // Non-spare bits are payload bits
+      auto numPayloadBits = payloadBits.countSetBits();
+
+      if (numPayloadBits >= 32) {
+	// Lots of payload bits!!  We only need one extra tag value
+	payloadTagValues += 1;
+      } else {
+	// We may need multiple tag values to cover all the non-payload cases
+	auto numNonPayloadCasesPerTag = 1ULL << numPayloadBits;
+	auto numNonPayloadCases = getNumCases() - NumEffectivePayloadCases;
+	payloadTagValues += (numNonPayloadCases + numNonPayloadCasesPerTag - 1) / numNonPayloadCasesPerTag;
+      }
     }
     int payloadTagBits = 0;
     while (payloadTagValues > 0) {

validation-test/Reflection/reflect_Enum_values10.swift

@@ -0,0 +1,66 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values10
+// RUN: %target-codesign %t/reflect_Enum_values10
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values10 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+protocol P : AnyObject {
+}
+
+class C : P {
+  var a: Int
+  var b: Int
+  init() { a = 0; b = 0; }
+}
+
+// MemoryLayout<B>.size == 8
+enum B {
+case a(C)
+case b(C)
+}
+
+reflect(enumValue: B.a(C()))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values10.B)
+// CHECKALL-NEXT: Value: .a(_)
+
+reflect(enumValue: B.b(C()))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values10.B)
+// CHECKALL-NEXT: Value: .b(_)
+
+// MemoryLayout<Q>.size == 16
+// MemoryLayout<P>.size == 16
+enum Q {
+case a(P)
+case b(P)
+}
+
+reflect(enumValue: Q.a(C()))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values10.Q)
+// CHECKALL-NEXT: Value: .a(_)
+
+reflect(enumValue: Q.b(C()))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values10.Q)
+// CHECKALL-NEXT: Value: .b(_)
+
+doneReflecting()
+
+// CHECKALL: Done.
+

validation-test/Reflection/reflect_Enum_values11.swift

@@ -0,0 +1,114 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values11
+// RUN: %target-codesign %t/reflect_Enum_values11
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values11 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+public enum A1 {
+  case item1
+}
+public enum A2 {
+  case item1
+}
+public enum A3 {
+  case item1
+}
+public enum A4 {
+  case item1
+}
+public enum A5 {
+  case item1
+}
+public enum A6 {
+  case item1
+  case item2
+}
+public enum A7 {
+  case item1
+  case item2
+}
+
+// MemoryLayout<Request>.size = 1
+public enum Request {
+case a1(A1)
+case a2(A2)
+case a3(A3)
+case a4(A4)
+case a5(A5)
+case a6(A6)
+case a7(A7)
+}
+
+reflect(enumValue: Request.a1(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a1(.item1)
+
+reflect(enumValue: Request.a2(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a2(.item1)
+
+reflect(enumValue: Request.a3(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a3(.item1)
+
+reflect(enumValue: Request.a4(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a4(.item1)
+
+reflect(enumValue: Request.a5(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a5(.item1)
+
+reflect(enumValue: Request.a6(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a6(.item1)
+
+reflect(enumValue: Request.a6(.item2))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a6(.item2)
+
+reflect(enumValue: Request.a7(.item1))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a7(.item1)
+
+reflect(enumValue: Request.a7(.item2))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values11.Request)
+// CHECKALL-NEXT: Value: .a7(.item2)
+
+doneReflecting()
+
+// CHECKALL: Done.
+

validation-test/Reflection/reflect_Enum_values7.swift

@@ -0,0 +1,42 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values7
+// RUN: %target-codesign %t/reflect_Enum_values7
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values7 | tee /dev/stderr | %FileCheck %s --dump-input=fail
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+public struct MyError : Error {}
+
+// MemoryLayout<MPEWithClosurePayload>.size == 17
+// MemoryLayout<()->Void>.size == 16
+enum MPEWithClosurePayload {
+    case int(()->Void)
+    case closure(() -> Void)
+}
+
+// MemoryLayout<E>.size == 17
+// enum E { case a(()->Void); case b(()->Void); }
+
+reflect(enumValue: MPEWithClosurePayload.int({}))
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_values7.MPEWithClosurePayload)
+// CHECK-NEXT: Value: .int(_)
+
+reflect(enumValue: MPEWithClosurePayload.closure({}))
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_values7.MPEWithClosurePayload)
+// CHECK-NEXT: Value: .closure(_)
+
+doneReflecting()
+
+// CHECK: Done.
+

validation-test/Reflection/reflect_Enum_values8.swift

@@ -0,0 +1,47 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values8
+// RUN: %target-codesign %t/reflect_Enum_values8
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values8 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+enum B {
+case a(Int)
+case b(Float)
+}
+
+enum A_Opt {
+case a(B?)
+case other(Int)
+}
+
+reflect(enumValue: A_Opt.a(.none))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values8.A_Opt)
+// CHECKALL-NEXT: Value: .a(.none)
+
+reflect(enumValue: A_Opt.a(.a(0)))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values8.A_Opt)
+// CHECKALL-NEXT: Value: .a(.some(.a(_)))
+
+reflect(enumValue: A_Opt.a(.b(0.0)))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values8.A_Opt)
+// CHECKALL-NEXT: Value: .a(.some(.b(_)))
+
+doneReflecting()
+
+// CHECKALL: Done.
+

validation-test/Reflection/reflect_Enum_values9.swift

@@ -0,0 +1,41 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values9
+// RUN: %target-codesign %t/reflect_Enum_values9
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values9 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+enum E : Error {
+  case e
+}
+
+// MemoryLayout<B>.size == 8
+// MemoryLayout<Error>.size == 8
+enum B {
+case a(Error)
+case b(Error)
+}
+
+reflect(enumValue: B.a(E.e))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values9.B)
+// CHECKALL-NEXT: Value: .a(_)
+
+reflect(enumValue: B.b(E.e))
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values9.B)
+// CHECKALL-NEXT: Value: .b(_)
+
+doneReflecting()
+
+// CHECKALL: Done.
+