Merge pull request #22775 from slavapestov/resilient-witness-table-cleanup

Resilient witness table emission cleanup

Author: slavapestov

include/swift/ABI/Metadata.h

@@ -2039,7 +2039,8 @@ struct TargetGenericWitnessTable {
     return WitnessTablePrivateSizeInWordsAndRequiresInstantiation >> 1;
   }
 
-  /// Whether the witness table is known to require instantiation.
+  /// This bit doesn't really mean anything. Currently, the compiler always
+  /// sets it when emitting a generic witness table.
   uint16_t requiresInstantiation() const {
     return WitnessTablePrivateSizeInWordsAndRequiresInstantiation & 0x01;
   }

lib/IRGen/ConformanceDescription.h

@@ -51,9 +51,6 @@ class ConformanceDescription {
   /// Whether this witness table requires runtime specialization.
   const unsigned requiresSpecialization : 1;
 
-  /// Whether this witness table contains dependent associated type witnesses.
-  const unsigned hasDependentAssociatedTypeWitnesses : 1;
-
   /// The instantiation function, to be run at the end of witness table
   /// instantiation.
   llvm::Constant *instantiationFn = nullptr;
@@ -66,13 +63,11 @@ class ConformanceDescription {
                          llvm::Constant *pattern,
                          uint16_t witnessTableSize,
                          uint16_t witnessTablePrivateSize,
-                         bool requiresSpecialization,
-                         bool hasDependentAssociatedTypeWitnesses)
+                         bool requiresSpecialization)
     : conformance(conformance), wtable(wtable), pattern(pattern),
       witnessTableSize(witnessTableSize),
       witnessTablePrivateSize(witnessTablePrivateSize),
-      requiresSpecialization(requiresSpecialization),
-      hasDependentAssociatedTypeWitnesses(hasDependentAssociatedTypeWitnesses)
+      requiresSpecialization(requiresSpecialization)
   {
   }
 };

lib/IRGen/GenProto.cpp

@@ -867,7 +867,6 @@ static bool hasDependentTypeWitness(
 static bool isDependentConformance(
               IRGenModule &IGM,
               const RootProtocolConformance *rootConformance,
-              bool considerResilience,
               llvm::SmallPtrSet<const NormalProtocolConformance *, 4> &visited){
   // Self-conformances are never dependent.
   auto conformance = dyn_cast<NormalProtocolConformance>(rootConformance);
@@ -880,7 +879,7 @@ static bool isDependentConformance(
     return false;
 
   // If the conformance is resilient, this is always true.
-  if (considerResilience && IGM.isResilientConformance(conformance))
+  if (IGM.isResilientConformance(conformance))
     return true;
 
   // Check whether any of the conformances are dependent.
@@ -899,7 +898,6 @@ static bool isDependentConformance(
         isDependentConformance(IGM,
                                assocConformance.getConcrete()
                                  ->getRootConformance(),
-                               considerResilience,
                                visited))
       return true;
   }
@@ -916,11 +914,9 @@ static bool isDependentConformance(
 /// Is there anything about the given conformance that requires witness
 /// tables to be dependently-generated?
 bool IRGenModule::isDependentConformance(
-    const RootProtocolConformance *conformance,
-    bool considerResilience) {
+    const RootProtocolConformance *conformance) {
   llvm::SmallPtrSet<const NormalProtocolConformance *, 4> visited;
-  return ::isDependentConformance(*this, conformance, considerResilience,
-                                  visited);
+  return ::isDependentConformance(*this, conformance, visited);
 }
 
 static bool isSynthesizedNonUnique(const RootProtocolConformance *conformance) {
@@ -1140,7 +1136,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
     // offsets, with conditional conformances closest to 0.
     unsigned NextPrivateDataIndex = 0;
     bool ResilientConformance;
-    bool RequiresSpecialization = false;
 
     const ProtocolInfo &PI;
 
@@ -1162,21 +1157,14 @@ class AccessorConformanceInfo : public ConformanceInfo {
           PI(IGM.getProtocolInfo(SILWT->getConformance()->getProtocol(),
                                  (ResilientConformance
                                   ? ProtocolInfoKind::RequirementSignature
-                                  : ProtocolInfoKind::Full))) {
-      // If the conformance is resilient, we require runtime instantiation.
-      if (ResilientConformance)
-        RequiresSpecialization = true;
-    }
+                                  : ProtocolInfoKind::Full))) {}
 
     /// The number of entries in the witness table.
     unsigned getTableSize() const { return TableSize; }
 
     /// The number of private entries in the witness table.
     unsigned getTablePrivateSize() const { return NextPrivateDataIndex; }
 
-    /// Whether this witness table must be specialized at runtime.
-    bool requiresSpecialization() const { return RequiresSpecialization; }
-
     /// The top-level entry point.
     void build();
 
@@ -1226,7 +1214,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
       }
 
       // Otherwise, we'll need to derive it at instantiation time.
-      RequiresSpecialization = true;
       SpecializedBaseConformances.push_back({Table.size(), &conf});
       Table.addNullPointer(IGM.Int8PtrTy);
     }
@@ -1293,8 +1280,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
       auto associate =
         Conformance.getTypeWitness(requirement.getAssociation(), nullptr)
           ->getCanonicalType();
-      if (associate->hasTypeParameter())
-        RequiresSpecialization = true;
       llvm::Constant *witness =
           IGM.getAssociatedTypeWitness(associate, /*inProtocolContext=*/false);
       Table.addBitCast(witness, IGM.Int8PtrTy);
@@ -1319,9 +1304,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
           requirement.getAssociation(),
           requirement.getAssociatedRequirement());
 
-      if (requirement.getAssociation()->hasTypeParameter())
-        RequiresSpecialization = true;
-
 #ifndef NDEBUG
       assert(entry.getKind() == SILWitnessTable::AssociatedTypeProtocol
              && "sil witness table does not match protocol");
@@ -1372,7 +1354,6 @@ class AccessorConformanceInfo : public ConformanceInfo {
 
     /// Allocate another word of private data storage in the conformance table.
     unsigned getNextPrivateDataIndex() {
-      RequiresSpecialization = true;
       return NextPrivateDataIndex++;
     }
 
@@ -1911,7 +1892,7 @@ namespace {
       // WitnessTablePrivateSizeInWordsAndRequiresInstantiation
       B.addInt(IGM.Int16Ty,
                (Description.witnessTablePrivateSize << 1) |
-                Description.hasDependentAssociatedTypeWitnesses);
+                Description.requiresSpecialization);
       // Instantiation function
       B.addRelativeAddressOrNull(Description.instantiationFn);
       // Private data
@@ -2065,13 +2046,12 @@ IRGenModule::getConformanceInfo(const ProtocolDecl *protocol,
 
   const ConformanceInfo *info;
   // If the conformance is dependent in any way, we need to unique it.
-  // TODO: maybe this should apply whenever it's out of the module?
-  // TODO: actually enable this
+  //
   // FIXME: Both implementations of ConformanceInfo are trivially-destructible,
   // so in theory we could allocate them on a BumpPtrAllocator. But there's not
   // a good one for us to use. (The ASTContext's outlives the IRGenModule in
   // batch mode.)
-  if (isDependentConformance(rootConformance, /*considerResilience=*/true) ||
+  if (isDependentConformance(rootConformance) ||
       // Foreign types need to go through the accessor to unique the witness
       // table.
       isSynthesizedNonUnique(rootConformance)) {
@@ -2143,13 +2123,13 @@ void IRGenModule::emitSILWitnessTable(SILWitnessTable *wt) {
   // Produce the initializer value.
   auto initializer = wtableContents.finishAndCreateFuture();
 
+  bool isDependent = isDependentConformance(conf);
+
   llvm::GlobalVariable *global = nullptr;
   unsigned tableSize;
   if (!isResilientConformance(conf)) {
-    bool isDependent =
-      isDependentConformance(conf, /*considerResilience=*/false);
     global = cast<llvm::GlobalVariable>(
-      isDependent
+      (isDependent && conf->getDeclContext()->isGenericContext())
         ? getAddrOfWitnessTablePattern(cast<NormalProtocolConformance>(conf),
                                        initializer)
         : getAddrOfWitnessTable(conf, initializer));
@@ -2165,10 +2145,7 @@ void IRGenModule::emitSILWitnessTable(SILWitnessTable *wt) {
   // descriptor.
   ConformanceDescription description(conf, wt, global, tableSize,
                                      wtableBuilder.getTablePrivateSize(),
-                                     wtableBuilder.requiresSpecialization(),
-                                     isDependentConformance(
-                                       conf,
-                                       /*considerResilience=*/false));
+                                     isDependent);
 
   // Build the instantiation function, we if need one.
   description.instantiationFn = wtableBuilder.buildInstantiationFunction();

lib/IRGen/IRGenModule.h

@@ -794,8 +794,7 @@ class IRGenModule {
 
   bool isResilientConformance(const NormalProtocolConformance *conformance);
   bool isResilientConformance(const RootProtocolConformance *root);
-  bool isDependentConformance(const RootProtocolConformance *conformance,
-                              bool considerResilience);
+  bool isDependentConformance(const RootProtocolConformance *conformance);
 
   Alignment getCappedAlignment(Alignment alignment);
 

test/IRGen/protocol_conformance_records.swift

@@ -22,8 +22,7 @@ public protocol Runcible {
 // -- witness table
 // CHECK-SAME:           @"$s28protocol_conformance_records15NativeValueTypeVAA8RuncibleAAWP"
 // -- flags
-// CHECK-SAME:           i32 0
-// CHECK-SAME:         },
+// CHECK-SAME:           i32 0 },
 public struct NativeValueType: Runcible {
   public func runce() {}
 }
@@ -36,8 +35,7 @@ public struct NativeValueType: Runcible {
 // -- witness table
 // CHECK-SAME:           @"$s28protocol_conformance_records15NativeClassTypeCAA8RuncibleAAWP"
 // -- flags
-// CHECK-SAME:           i32 0
-// CHECK-SAME:         },
+// CHECK-SAME:           i32 0 },
 public class NativeClassType: Runcible {
   public func runce() {}
 }
@@ -50,8 +48,7 @@ public class NativeClassType: Runcible {
 // -- witness table
 // CHECK-SAME:           @"$s28protocol_conformance_records17NativeGenericTypeVyxGAA8RuncibleAAWP"
 // -- flags
-// CHECK-SAME:           i32 0
-// CHECK-SAME:         },
+// CHECK-SAME:           i32 0 },
 public struct NativeGenericType<T>: Runcible {
   public func runce() {}
 }
@@ -87,6 +84,30 @@ extension Size: Runcible {
   public func runce() {}
 }
 
+// A non-dependent type conforming to a protocol with associated conformances
+// does not require a generic witness table.
+public protocol Simple {}
+
+public protocol AssociateConformance {
+  associatedtype X : Simple
+}
+
+public struct Other : Simple {}
+
+public struct Concrete : AssociateConformance {
+  public typealias X = Other
+}
+
+// CHECK-LABEL: @"$s28protocol_conformance_records8ConcreteVAA20AssociateConformanceAAMc" ={{ dllexport | protected | }}constant
+// -- protocol descriptor
+// CHECK-SAME:           @"$s28protocol_conformance_records20AssociateConformanceMp"
+// -- nominal type descriptor
+// CHECK-SAME:           @"$s28protocol_conformance_records8ConcreteVMn"
+// -- witness table
+// CHECK-SAME:           @"$s28protocol_conformance_records8ConcreteVAA20AssociateConformanceAAWP"
+// -- no flags are set, and no generic witness table follows
+// CHECK-SAME:           i32 0 }
+
 // CHECK-LABEL: @"\01l_protocols"
 // CHECK-SAME: @"$s28protocol_conformance_records8RuncibleMp"
 // CHECK-SAME: @"$s28protocol_conformance_records5SpoonMp"
@@ -136,7 +157,7 @@ extension Int : OtherResilientProtocol { }
 // -- witness table pattern
 // CHECK-SAME:           @"$s28protocol_conformance_records9DependentVyxGAA9AssociateAAWp"
 // -- flags
-// CHECK-SAME:           i32 0
+// CHECK-SAME:           i32 131072,
 // -- number of words in witness table
 // CHECK-SAME:           i16 2,
 // -- number of private words in witness table + bit for "needs instantiation"
@@ -151,5 +172,6 @@ extension Dependent : Associate {
 // CHECK-SAME: @"$s28protocol_conformance_records15NativeClassTypeCAA8RuncibleAAMc"
 // CHECK-SAME: @"$s28protocol_conformance_records17NativeGenericTypeVyxGAA8RuncibleAAMc"
 // CHECK-SAME: @"$s16resilient_struct4SizeV28protocol_conformance_records8RuncibleADMc"
+// CHECK-SAME: @"$s28protocol_conformance_records8ConcreteVAA20AssociateConformanceAAMc"
 // CHECK-SAME: @"$s28protocol_conformance_records17NativeGenericTypeVyxGAA5SpoonA2aERzlMc"
-// CHECK-SAME: @"$sSi18resilient_protocol22OtherResilientProtocol0B20_conformance_recordsMc"
+// CHECK-SAME: @"$sSi18resilient_protocol22OtherResilientProtocol0B20_conformance_recordsMc"

test/IRGen/protocol_resilience.sil

@@ -97,8 +97,8 @@ protocol InternalProtocol {
 // -- number of witness table entries
 // CHECK-SAME:   i16 0,
 
-// -- size of private area + 'requires instantiation' bit (not set)
-// CHECK-SAME:   i16 0,
+// -- size of private area + 'requires instantiation' bit
+// CHECK-SAME:   i16 1,
 
 // -- instantiator function
 // CHECK-SAME:   i32 0
@@ -127,8 +127,8 @@ protocol InternalProtocol {
 // -- number of witness table entries
 // CHECK-SAME:   i16 0,
 
-// -- size of private area + 'requires instantiation' bit (not set)
-// CHECK-SAME:   i16 0,
+// -- size of private area + 'requires instantiation' bit
+// CHECK-SAME:   i16 1,
 
 // -- instantiator function
 // CHECK-SAME:   i32 0

test/IRGen/protocol_resilience_descriptors.swift

@@ -60,10 +60,18 @@ public struct ConditionallyConforms<Element> { }
 public struct Y { }
 
 // CHECK-USAGE-LABEL: @"$s31protocol_resilience_descriptors1YV010resilient_A022OtherResilientProtocolAAMc" =
+// -- flags: has generic witness table
 // CHECK-USAGE-SAME: i32 131072,
+// -- number of witness table entries
 // CHECK-USAGE-SAME: i16 0,
-// CHECK-USAGE-SAME: i16 0,
-// CHECK-USAGE-SAME: i32 0
+// -- size of private area + 'requires instantiation' bit
+// CHECK-USAGE-SAME: i16 1,
+// -- instantiator function
+// CHECK-USAGE-SAME: i32 0,
+// -- private data area
+// CHECK-USAGE-SAME: {{@[0-9]+}}
+// --
+// CHECK-USAGE-SAME: }
 extension Y: OtherResilientProtocol { }
 
 // CHECK-USAGE: @"$s31protocol_resilience_descriptors29ConformsWithAssocRequirementsV010resilient_A008ProtocoleF12TypeDefaultsAAMc" =