DeadFunctionElimination: don’t eliminate public methods which are called via a thunk.

For this we need to store the linkage of the “original” method implementation in the vtable.
Otherwise DeadFunctionElimination thinks that the method implementation is not public but private (which is the linkage of the thunk).

The big part of this change is to extend SILVTable to store the linkage (+ serialization, printing, etc.).

fixes rdar://problem/29841635

Author: eeckstein

docs/SIL.rst

@@ -1028,7 +1028,7 @@ VTables
   decl ::= sil-vtable
   sil-vtable ::= 'sil_vtable' identifier '{' sil-vtable-entry* '}'
 
-  sil-vtable-entry ::= sil-decl-ref ':' sil-function-name
+  sil-vtable-entry ::= sil-decl-ref ':' sil-linkage? sil-function-name
 
 SIL represents dynamic dispatch for class methods using the `class_method`_,
 `super_method`_, and `dynamic_method`_ instructions. The potential destinations
@@ -1084,6 +1084,9 @@ visible through that class (in the example above, ``B``'s vtable references
 that can be used to look up overridden methods in the SIL vtable for a derived
 class (such as ``C.bas`` in ``C``'s vtable).
 
+In case the SIL function is a thunk, the function name is preceded with the
+linkage of the original implementing function.
+
 Witness Tables
 ~~~~~~~~~~~~~~
 ::

include/swift/SIL/SILVTable.h

@@ -42,7 +42,28 @@ class SILVTable : public llvm::ilist_node<SILVTable>,
 public:
   // TODO: Entry should include substitutions needed to invoke an overridden
   // generic base class method.
-  using Pair = std::pair<SILDeclRef, SILFunction*>;
+  struct Entry {
+
+    Entry() : Implementation(nullptr), Linkage(SILLinkage::Private) { }
+
+    Entry(SILDeclRef Method, SILFunction *Implementation, SILLinkage Linkage) :
+      Method(Method), Implementation(Implementation), Linkage(Linkage) { }
+
+    /// The declaration reference to the least-derived method visible through
+    /// the class.
+    SILDeclRef Method;
+
+    /// The function which implements the method for the class.
+    SILFunction *Implementation;
+
+    /// The linkage of the implementing function.
+    ///
+    /// This is usuallly the same as
+    ///   stripExternalFromLinkage(Implementation->getLinkage())
+    /// except if Implementation is a thunk (which has private or shared
+    /// linkage).
+    SILLinkage Linkage;
+  };
 
   // Disallow copying into temporary objects.
   SILVTable(const SILVTable &other) = delete;
@@ -56,10 +77,10 @@ class SILVTable : public llvm::ilist_node<SILVTable>,
   unsigned NumEntries;
 
   /// Tail-allocated SILVTable entries.
-  Pair Entries[1];
+  Entry Entries[1];
 
   /// Private constructor. Create SILVTables by calling SILVTable::create.
-  SILVTable(ClassDecl *c, ArrayRef<Pair> entries);
+  SILVTable(ClassDecl *c, ArrayRef<Entry> entries);
 
 public:
   ~SILVTable();
@@ -68,24 +89,24 @@ class SILVTable : public llvm::ilist_node<SILVTable>,
   /// The SILDeclRef keys should reference the most-overridden members available
   /// through the class.
   static SILVTable *create(SILModule &M, ClassDecl *Class,
-                           ArrayRef<Pair> Entries);
+                           ArrayRef<Entry> Entries);
 
   /// Return the class that the vtable represents.
   ClassDecl *getClass() const { return Class; }
 
   /// Return all of the method entries.
-  ArrayRef<Pair> getEntries() const { return {Entries, NumEntries}; }
+  ArrayRef<Entry> getEntries() const { return {Entries, NumEntries}; }
 
   /// Look up the implementation function for the given method.
   SILFunction *getImplementation(SILModule &M, SILDeclRef method) const;
 
   /// Removes entries from the vtable.
   /// \p predicate Returns true if the passed entry should be removed.
   template <typename Predicate> void removeEntries_if(Predicate predicate) {
-    Pair *end = std::remove_if(Entries, Entries + NumEntries,
-                               [&](Pair &entry) -> bool {
+    Entry *end = std::remove_if(Entries, Entries + NumEntries,
+                                [&](Entry &entry) -> bool {
       if (predicate(entry)) {
-        entry.second->decrementRefCount();
+        entry.Implementation->decrementRefCount();
         removeFromVTableCache(entry);
         return true;
       }
@@ -102,7 +123,7 @@ class SILVTable : public llvm::ilist_node<SILVTable>,
   void dump() const;
 
 private:
-  void removeFromVTableCache(Pair &entry);
+  void removeFromVTableCache(Entry &entry);
 };
 
 } // end swift namespace

include/swift/Serialization/ModuleFormat.h

@@ -54,7 +54,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// in source control, you should also update the comment to briefly
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
-const uint16_t VERSION_MINOR = 304; // Last change: Archetype generic env
+const uint16_t VERSION_MINOR = 305; // Last change: linkage in SILVTable::Entry
 
 using DeclID = PointerEmbeddedInt<unsigned, 31>;
 using DeclIDField = BCFixed<31>;

lib/Parse/ParseSIL.cpp

@@ -4272,7 +4272,7 @@ bool Parser::parseSILVTable() {
   Lexer::SILBodyRAII Tmp(*L);
   Scope S(this, ScopeKind::TopLevel);
   // Parse the entry list.
-  std::vector<SILVTable::Pair> vtableEntries;
+  std::vector<SILVTable::Entry> vtableEntries;
   if (Tok.isNot(tok::r_brace)) {
     do {
       SILDeclRef Ref;
@@ -4281,10 +4281,12 @@ bool Parser::parseSILVTable() {
       if (VTableState.parseSILDeclRef(Ref))
         return true;
       SILFunction *Func = nullptr;
+      Optional<SILLinkage> Linkage = SILLinkage::Private;
       if (Tok.is(tok::kw_nil)) {
         consumeToken();
       } else {
         if (parseToken(tok::colon, diag::expected_sil_vtable_colon) ||
+          parseSILLinkage(Linkage, *this) ||
           VTableState.parseSILIdentifier(FuncName, FuncLoc,
                                          diag::expected_sil_value_name))
         return true;
@@ -4293,8 +4295,10 @@ bool Parser::parseSILVTable() {
           diagnose(FuncLoc, diag::sil_vtable_func_not_found, FuncName);
           return true;
         }
+        if (!Linkage)
+          Linkage = stripExternalFromLinkage(Func->getLinkage());
       }
-      vtableEntries.emplace_back(Ref, Func);
+      vtableEntries.emplace_back(Ref, Func, Linkage.getValue());
     } while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof));
   }
 

lib/SIL/Linker.cpp

@@ -139,7 +139,7 @@ SILVTable *SILLinkerVisitor::processClassDecl(const ClassDecl *C) {
   // Otherwise, add all the vtable functions in Vtbl to the function
   // processing list...
   for (auto &E : Vtbl->getEntries())
-    Worklist.push_back(E.second);
+    Worklist.push_back(E.Implementation);
 
   // And then transitively deserialize all SIL referenced by those functions.
   process();
@@ -162,9 +162,9 @@ bool SILLinkerVisitor::linkInVTable(ClassDecl *D) {
   // for processing.
   bool Result = false;
   for (auto P : Vtbl->getEntries()) {
-    if (P.second->isExternalDeclaration()) {
+    if (P.Implementation->isExternalDeclaration()) {
       Result = true;
-      addFunctionToWorklist(P.second);
+      addFunctionToWorklist(P.Implementation);
     }
   }
   return Result;

lib/SIL/SILPrinter.cpp

@@ -2141,9 +2141,14 @@ void SILVTable::print(llvm::raw_ostream &OS, bool Verbose) const {
   OS << "sil_vtable " << getClass()->getName() << " {\n";
   for (auto &entry : getEntries()) {
     OS << "  ";
-    entry.first.print(OS);
-    OS << ": " << entry.second->getName()
-       << "\t// " << demangleSymbol(entry.second->getName()) << "\n";
+    entry.Method.print(OS);
+    OS << ": ";
+    if (entry.Linkage !=
+        stripExternalFromLinkage(entry.Implementation->getLinkage())) {
+      OS << getLinkageString(entry.Linkage);
+    }
+    OS << entry.Implementation->getName()
+       << "\t// " << demangleSymbol(entry.Implementation->getName()) << "\n";
   }
   OS << "}\n\n";
 }

lib/SIL/SILVTable.cpp

@@ -24,18 +24,18 @@
 using namespace swift;
 
 SILVTable *SILVTable::create(SILModule &M, ClassDecl *Class,
-                             ArrayRef<Pair> Entries) {
+                             ArrayRef<Entry> Entries) {
   // SILVTable contains one element declared in Entries.  We must allocate
   // space for it, because its default ctor will write to it.
   unsigned NumTailElements = std::max((unsigned)Entries.size(), 1U)-1;
-  void *buf = M.allocate(sizeof(SILVTable) + sizeof(Pair) * NumTailElements,
+  void *buf = M.allocate(sizeof(SILVTable) + sizeof(Entry) * NumTailElements,
                          alignof(SILVTable));
   SILVTable *vt = ::new (buf) SILVTable(Class, Entries);
   M.vtables.push_back(vt);
   M.VTableMap[Class] = vt;
   // Update the Module's cache with new vtable + vtable entries:
-  for (auto &entry : Entries) {
-    M.VTableEntryCache.insert({{vt, entry.first}, entry.second});
+  for (const Entry &entry : Entries) {
+    M.VTableEntryCache.insert({{vt, entry.Method}, entry.Implementation});
   }
   return vt;
 }
@@ -44,33 +44,33 @@ SILFunction *
 SILVTable::getImplementation(SILModule &M, SILDeclRef method) const {
   SILDeclRef m = method;
   do {
-    auto entry = M.VTableEntryCache.find({this, m});
-    if (entry != M.VTableEntryCache.end()) {
-      return (*entry).second;
+    auto entryIter = M.VTableEntryCache.find({this, m});
+    if (entryIter != M.VTableEntryCache.end()) {
+      return (*entryIter).second;
     }
   } while ((m = m.getOverridden()));
   return nullptr;
 }
 
-void SILVTable::removeFromVTableCache(Pair &entry) {
-  SILModule &M = entry.second->getModule();
-  M.VTableEntryCache.erase({this, entry.first});
+void SILVTable::removeFromVTableCache(Entry &entry) {
+  SILModule &M = entry.Implementation->getModule();
+  M.VTableEntryCache.erase({this, entry.Method});
 }
 
-SILVTable::SILVTable(ClassDecl *c, ArrayRef<Pair> entries)
+SILVTable::SILVTable(ClassDecl *c, ArrayRef<Entry> entries)
   : Class(c), NumEntries(entries.size())
 {
-  memcpy(Entries, entries.begin(), sizeof(Pair) * NumEntries);
+  memcpy(Entries, entries.begin(), sizeof(Entry) * NumEntries);
 
   // Bump the reference count of functions referenced by this table.
-  for (auto entry : getEntries()) {
-    entry.second->incrementRefCount();
+  for (const Entry &entry : getEntries()) {
+    entry.Implementation->incrementRefCount();
   }
 }
 
 SILVTable::~SILVTable() {
   // Drop the reference count of functions referenced by this table.
-  for (auto entry : getEntries()) {
-    entry.second->decrementRefCount();
+  for (const Entry &entry : getEntries()) {
+    entry.Implementation->decrementRefCount();
   }
 }

lib/SIL/SILVerifier.cpp

@@ -3592,17 +3592,17 @@ void SILVTable::verify(const SILModule &M) const {
 #ifndef NDEBUG
   for (auto &entry : getEntries()) {
     // All vtable entries must be decls in a class context.
-    assert(entry.first.hasDecl() && "vtable entry is not a decl");
-    auto baseInfo = M.Types.getConstantInfo(entry.first);
-    ValueDecl *decl = entry.first.getDecl();
+    assert(entry.Method.hasDecl() && "vtable entry is not a decl");
+    auto baseInfo = M.Types.getConstantInfo(entry.Method);
+    ValueDecl *decl = entry.Method.getDecl();
 
     assert((!isa<FuncDecl>(decl)
             || !cast<FuncDecl>(decl)->isObservingAccessor())
            && "observing accessors shouldn't have vtable entries");
 
     // For ivar destroyers, the decl is the class itself.
     ClassDecl *theClass;
-    if (entry.first.kind == SILDeclRef::Kind::IVarDestroyer)
+    if (entry.Method.kind == SILDeclRef::Kind::IVarDestroyer)
       theClass = dyn_cast<ClassDecl>(decl);
     else
       theClass = dyn_cast<ClassDecl>(decl->getDeclContext());
@@ -3622,22 +3622,22 @@ void SILVTable::verify(const SILModule &M) const {
     assert(c && "vtable entry must refer to a member of the vtable's class");
 
     // All function vtable entries must be at their natural uncurry level.
-    assert(!entry.first.isCurried && "vtable entry must not be curried");
+    assert(!entry.Method.isCurried && "vtable entry must not be curried");
 
     // Foreign entry points shouldn't appear in vtables.
-    assert(!entry.first.isForeign && "vtable entry must not be foreign");
+    assert(!entry.Method.isForeign && "vtable entry must not be foreign");
 
     // The vtable entry must be ABI-compatible with the overridden vtable slot.
     SmallString<32> baseName;
     {
       llvm::raw_svector_ostream os(baseName);
-      entry.first.print(os);
+      entry.Method.print(os);
     }
 
-    SILVerifier(*entry.second)
+    SILVerifier(*entry.Implementation)
       .requireABICompatibleFunctionTypes(
                     baseInfo.getSILType().castTo<SILFunctionType>(),
-                    entry.second->getLoweredFunctionType(),
+                    entry.Implementation->getLoweredFunctionType(),
                     "vtable entry for " + baseName + " must be ABI-compatible");
   }
 #endif
@@ -3747,9 +3747,9 @@ void SILModule::verify() const {
     vt.verify(*this);
     // Check if there is a cache entry for each vtable entry
     for (auto entry : vt.getEntries()) {
-      if (VTableEntryCache.find({&vt, entry.first}) == VTableEntryCache.end()) {
-        llvm::errs() << "Vtable entry for function: " << entry.second->getName()
-                     << "not in cache!\n";
+      if (VTableEntryCache.find({&vt, entry.Method}) == VTableEntryCache.end()) {
+        llvm::errs() << "Vtable entry for function: "
+                     << entry.Implementation->getName() << "not in cache!\n";
         assert(false && "triggering standard assertion failure routine");
       }
       EntriesSZ++;

lib/SILGen/SILGen.h

@@ -172,7 +172,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
   /// diverges from the overridden base method. If no thunking is needed,
   /// returns a static reference to the derived method.
   SILFunction *emitVTableMethod(SILDeclRef derived,
-                                SILDeclRef base);
+                                SILDeclRef base,
+                                SILLinkage &implLinkage);
 
   /// True if a function has been emitted for a given SILDeclRef.
   bool hasFunction(SILDeclRef constant);

lib/SILGen/SILGenFunction.h

@@ -651,6 +651,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   /// \param inputSubstType Formal AST type of base class method
   /// \param outputSubstType Formal AST type of derived class method
   void emitVTableThunk(SILDeclRef derived,
+                       SILFunction *implFn,
                        AbstractionPattern inputOrigType,
                        CanAnyFunctionType inputSubstType,
                        CanAnyFunctionType outputSubstType);

lib/SILGen/SILGenPoly.cpp

@@ -2891,6 +2891,7 @@ SILGenFunction::emitTransformedValue(SILLocation loc, RValue &&v,
 
 void
 SILGenFunction::emitVTableThunk(SILDeclRef derived,
+                                SILFunction *implFn,
                                 AbstractionPattern inputOrigType,
                                 CanAnyFunctionType inputSubstType,
                                 CanAnyFunctionType outputSubstType) {
@@ -2902,7 +2903,6 @@ SILGenFunction::emitVTableThunk(SILDeclRef derived,
   cleanupLoc.markAutoGenerated();
   Scope scope(Cleanups, cleanupLoc);
 
-  auto implFn = SGM.getFunction(derived, NotForDefinition);
   auto fTy = implFn->getLoweredFunctionType();
 
   ArrayRef<Substitution> subs;