Merge pull request #21476 from slavapestov/sil-witness-table-cleanup

Clean up SIL witness table deserialization

Author: slavapestov

include/swift/SIL/SILModule.h

@@ -570,11 +570,6 @@ class SILModule {
   /// hierarchy of \p Class.
   SILFunction *lookUpFunctionInVTable(ClassDecl *Class, SILDeclRef Member);
 
-  // Given a protocol conformance, attempt to create a witness table declaration
-  // for it.
-  SILWitnessTable *
-  createWitnessTableDeclaration(ProtocolConformance *C, SILLinkage linkage);
-
   // Given a protocol, attempt to create a default witness table declaration
   // for it.
   SILDefaultWitnessTable *

lib/IRGen/IRGenModule.cpp

@@ -769,7 +769,7 @@ bool IRGenerator::canEmitWitnessTableLazily(SILWitnessTable *wt) {
 }
 
 void IRGenerator::addLazyWitnessTable(const ProtocolConformance *Conf) {
-  if (SILWitnessTable *wt = SIL.lookUpWitnessTable(Conf)) {
+  if (auto *wt = SIL.lookUpWitnessTable(Conf, /*deserializeLazily=*/false)) {
     // Add it to the queue if it hasn't already been put there.
     if (canEmitWitnessTableLazily(wt) &&
         LazilyEmittedWitnessTables.insert(wt).second) {

lib/ParseSIL/ParseSIL.cpp

@@ -6184,6 +6184,8 @@ bool SILParserTUState::parseSILWitnessTable(Parser &P) {
 
   if (!wt)
     wt = SILWitnessTable::create(M, *Linkage, theConformance);
+  else
+    wt->setLinkage(*Linkage);
   wt->convertToDefinition(witnessEntries, conditionalConformances,
                           isSerialized);
   BodyScope.reset();

lib/SIL/Linker.cpp

@@ -200,7 +200,7 @@ static bool mustDeserializeProtocolConformance(SILModule &M,
 
 void SILLinkerVisitor::visitProtocolConformance(
     ProtocolConformanceRef ref, const Optional<SILDeclRef> &Member) {
-  // If an abstract protocol conformance was passed in, just return false.
+  // If an abstract protocol conformance was passed in, do nothing.
   if (ref.isAbstract())
     return;
 
@@ -211,31 +211,16 @@ void SILLinkerVisitor::visitProtocolConformance(
 
   if (!VisitedConformances.insert(C).second)
     return;
-  
-  SILWitnessTable *WT = Mod.lookUpWitnessTable(C, true);
-
-  // If we don't find any witness table for the conformance, bail and return
-  // false.
-  if (!WT) {
-    Mod.createWitnessTableDeclaration(
-        C, getLinkageForProtocolConformance(
-               C->getRootNormalConformance(), NotForDefinition));
-
-    // Adding the declaration may allow us to now deserialize the body.
-    // Force the body if we must deserialize this witness table.
-    if (mustDeserialize) {
-      WT = Mod.lookUpWitnessTable(C, true);
-      assert(WT && WT->isDefinition()
-             && "unable to deserialize witness table when we must?!");
-    } else {
-      return;
-    }
-  }
+
+  auto *WT = Mod.lookUpWitnessTable(C, mustDeserialize);
 
   // If the looked up witness table is a declaration, there is nothing we can
-  // do here. Just bail and return false.
-  if (WT->isDeclaration())
+  // do here.
+  if (WT == nullptr || WT->isDeclaration()) {
+    assert(!mustDeserialize &&
+           "unable to deserialize witness table when we must?!");
     return;
+  }
 
   auto maybeVisitRelatedConformance = [&](ProtocolConformanceRef c) {
     // Formally all conformances referenced by a used conformance are used.

lib/SIL/SILInstructions.cpp

@@ -312,16 +312,6 @@ VarDecl *DebugValueAddrInst::getDecl() const {
   return getLoc().getAsASTNode<VarDecl>();
 }
 
-static void declareWitnessTable(SILModule &Mod,
-                                ProtocolConformanceRef conformanceRef) {
-  if (conformanceRef.isAbstract()) return;
-  auto C = conformanceRef.getConcrete();
-  if (!Mod.lookUpWitnessTable(C, false))
-    Mod.createWitnessTableDeclaration(C,
-        getLinkageForProtocolConformance(C->getRootNormalConformance(),
-                                         NotForDefinition));
-}
-
 AllocExistentialBoxInst *AllocExistentialBoxInst::create(
     SILDebugLocation Loc, SILType ExistentialType, CanType ConcreteType,
     ArrayRef<ProtocolConformanceRef> Conformances,
@@ -333,8 +323,6 @@ AllocExistentialBoxInst *AllocExistentialBoxInst::create(
   SILModule &Mod = F->getModule();
   auto Size = totalSizeToAlloc<swift::Operand>(TypeDependentOperands.size());
   auto Buffer = Mod.allocateInst(Size, alignof(AllocExistentialBoxInst));
-  for (ProtocolConformanceRef C : Conformances)
-    declareWitnessTable(Mod, C);
   return ::new (Buffer) AllocExistentialBoxInst(Loc,
                                                 ExistentialType,
                                                 ConcreteType,
@@ -1633,7 +1621,6 @@ WitnessMethodInst::create(SILDebugLocation Loc, CanType LookupType,
   auto Size = totalSizeToAlloc<swift::Operand>(TypeDependentOperands.size());
   auto Buffer = Mod.allocateInst(Size, alignof(WitnessMethodInst));
 
-  declareWitnessTable(Mod, Conformance);
   return ::new (Buffer) WitnessMethodInst(Loc, LookupType, Conformance, Member,
                                           Ty, TypeDependentOperands);
 }
@@ -1667,8 +1654,6 @@ InitExistentialAddrInst *InitExistentialAddrInst::create(
       totalSizeToAlloc<swift::Operand>(1 + TypeDependentOperands.size());
   void *Buffer = Mod.allocateInst(size,
                                   alignof(InitExistentialAddrInst));
-  for (ProtocolConformanceRef C : Conformances)
-    declareWitnessTable(Mod, C);
   return ::new (Buffer) InitExistentialAddrInst(Loc, Existential,
                                                 TypeDependentOperands,
                                                 ConcreteType,
@@ -1688,9 +1673,6 @@ InitExistentialValueInst *InitExistentialValueInst::create(
       totalSizeToAlloc<swift::Operand>(1 + TypeDependentOperands.size());
 
   void *Buffer = Mod.allocateInst(size, alignof(InitExistentialRefInst));
-  for (ProtocolConformanceRef C : Conformances)
-    declareWitnessTable(Mod, C);
-
   return ::new (Buffer)
       InitExistentialValueInst(Loc, ExistentialType, ConcreteType, Instance,
                                 TypeDependentOperands, Conformances);
@@ -1711,9 +1693,6 @@ InitExistentialRefInst::create(SILDebugLocation Loc, SILType ExistentialType,
 
   void *Buffer = Mod.allocateInst(size,
                                   alignof(InitExistentialRefInst));
-  for (ProtocolConformanceRef C : Conformances)
-    declareWitnessTable(Mod, C);
-
   return ::new (Buffer) InitExistentialRefInst(Loc, ExistentialType,
                                                ConcreteType,
                                                Instance,
@@ -1746,9 +1725,6 @@ InitExistentialMetatypeInst *InitExistentialMetatypeInst::create(
       1 + TypeDependentOperands.size(), conformances.size());
 
   void *buffer = M.allocateInst(size, alignof(InitExistentialMetatypeInst));
-  for (ProtocolConformanceRef conformance : conformances)
-    declareWitnessTable(M, conformance);
-
   return ::new (buffer) InitExistentialMetatypeInst(
       Loc, existentialMetatypeType, metatype,
       TypeDependentOperands, conformances);

lib/SIL/SILModule.cpp

@@ -143,19 +143,6 @@ void SILModule::deallocateInst(SILInstruction *I) {
   AlignedFree(I);
 }
 
-SILWitnessTable *
-SILModule::createWitnessTableDeclaration(ProtocolConformance *C,
-                                         SILLinkage linkage) {
-  // If we are passed in a null conformance (a valid value), just return nullptr
-  // since we cannot map a witness table to it.
-  if (!C)
-    return nullptr;
-
-  // Extract the root conformance.
-  auto rootC = C->getRootConformance();
-  return SILWitnessTable::create(*this, linkage, rootC);
-}
-
 SILWitnessTable *
 SILModule::lookUpWitnessTable(ProtocolConformanceRef C,
                               bool deserializeLazily) {
@@ -172,6 +159,8 @@ SILModule::lookUpWitnessTable(const ProtocolConformance *C,
                               bool deserializeLazily) {
   assert(C && "null conformance passed to lookUpWitnessTable");
 
+  SILWitnessTable *wtable;
+
   auto rootC = C->getRootConformance();
   // Attempt to lookup the witness table from the table.
   auto found = WitnessTableMap.find(rootC);
@@ -189,16 +178,24 @@ SILModule::lookUpWitnessTable(const ProtocolConformance *C,
              "Found witness table that is not"
              " in the witness table lookup cache.");
 #endif
-    return nullptr;
-  }
 
-  SILWitnessTable *wtable = found->second;
-  assert(wtable != nullptr && "Should never map a conformance to a null witness"
-                          " table.");
+    // If we don't have a witness table and we're not going to try
+    // deserializing it, do not create a declaration.
+    if (!deserializeLazily)
+      return nullptr;
 
-  // If we have a definition, return it.
-  if (wtable->isDefinition())
-    return wtable;
+    auto linkage = getLinkageForProtocolConformance(rootC, NotForDefinition);
+    wtable = SILWitnessTable::create(*this, linkage,
+                                 const_cast<RootProtocolConformance *>(rootC));
+  } else {
+    wtable = found->second;
+    assert(wtable != nullptr && "Should never map a conformance to a null witness"
+                            " table.");
+
+    // If we have a definition, return it.
+    if (wtable->isDefinition())
+      return wtable;
+  }
 
   // If the module is at or past the Lowered stage, then we can't do any
   // further deserialization, since pre-IRGen SIL lowering changes the types

lib/SIL/SILVerifier.cpp

@@ -2493,8 +2493,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       auto conformance = AMI->getConformance().getConcrete();
       require(conformance->getType()->isEqual(AMI->getLookupType()),
               "concrete type lookup requires conformance that matches type");
-      require(AMI->getModule().lookUpWitnessTable(conformance, false),
-              "Could not find witness table for conformance");
     }
 
     require(AMI->getMember().requiresNewWitnessTableEntry(),
@@ -3204,13 +3202,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       require(conformances[i].getRequirement() == protocols[i]->getDecl(),
               "init_existential instruction must have conformances in "
               "proper order");
-
-      if (conformances[i].isConcrete()) {
-        auto conformance = conformances[i].getConcrete();
-        require(F.getModule().lookUpWitnessTable(conformance, false),
-                "Could not find witness table for conformance.");
-
-      }
     }
   }
 

test/IRGen/existential_metatypes.sil

@@ -109,10 +109,10 @@ bb3:
   return %9 : $()
 }
 
-sil_witness_table Int: Kindable module existential_metatypes {
+sil_witness_table public_external Int: Kindable module existential_metatypes {
   method #Kindable.kind!getter.1: @int_kind_getter_witness
 }
 
-sil_witness_table Float: Kindable module existential_metatypes {
+sil_witness_table public_external Float: Kindable module existential_metatypes {
   method #Kindable.kind!getter.1: @float_kind_getter_witness
 }

test/SIL/Parser/witness_table_redeclare.sil

@@ -0,0 +1,12 @@
+// RUN: %target-sil-opt %s -module-name=witness_table_redeclare | %target-sil-opt -module-name=witness_table_redeclare | %FileCheck %s
+
+protocol P {}
+
+struct S : P {}
+
+sil_witness_table S: P module witness_table_redeclare
+
+sil_witness_table S: P module witness_table_redeclare {}
+
+// CHECK-LABEL: sil_witness_table S: P module witness_table_redeclare {
+// CHECK-NEXT: }

test/SIL/Serialization/Inputs/init_existential_inst_deserializes_witness_tables_input.swift

@@ -13,6 +13,7 @@ public struct X : P {
   public init() {}
 }
 
+@inlinable
 public func whatShouldIDo(_ p : P) {
   p.doSomething()
 }

test/SILOptimizer/dead_witness_module.swift

@@ -5,9 +5,11 @@
 // DeadFunctionElimination may not remove a method from a witness table which
 // is imported from another module.
 
+// FIXME: Ever since @usableFromInline began to be enforced, this test did not
+// test anything useful, and now the witness table is never deserialized at all.
+
 import TestModule
 
 testit(MyStruct())
 
-// CHECK: sil_witness_table public_external MyStruct: Proto module TestModule
-// CHECK-NEXT: method #Proto.confx!1: {{.*}} : @$s{{.*}}confx{{.*}}FTW
+// CHECK-NOT: sil_witness_table MyStruct: Proto module TestModule