SIL: always update the linkage of a function with the serialized linkage in the swiftmodule.

The cross-module-optimization can change the linkage of a function to public. Then the SILLinkage is "out of sync" with the linkage derived from the AST. We need to make sure to read the correct SILLinkage from the module file.

Author: eeckstein

include/swift/SIL/SILModule.h

@@ -526,6 +526,13 @@ class SILModule {
   /// succeeded, false otherwise.
   bool loadFunction(SILFunction *F);
 
+  /// Update the linkage of the SILFunction with the linkage of the serialized
+  /// function.
+  ///
+  /// The serialized SILLinkage can differ from the linkage derived from the
+  /// AST, e.g. cross-module-optimization can change the SIL linkages.
+  void updateFunctionLinkage(SILFunction *F);
+
   /// Attempt to link the SILFunction. Returns true if linking succeeded, false
   /// otherwise.
   ///

include/swift/Serialization/SerializedSILLoader.h

@@ -56,7 +56,7 @@ class SerializedSILLoader {
   }
   ~SerializedSILLoader();
 
-  SILFunction *lookupSILFunction(SILFunction *Callee);
+  SILFunction *lookupSILFunction(SILFunction *Callee, bool onlyUpdateLinkage);
   SILFunction *
   lookupSILFunction(StringRef Name, bool declarationOnly = false,
                     Optional<SILLinkage> linkage = None);

lib/SIL/Linker.cpp

@@ -114,6 +114,11 @@ void SILLinkerVisitor::maybeAddFunctionToWorklist(SILFunction *F) {
   // So try deserializing HiddenExternal functions too.
   if (F->getLinkage() == SILLinkage::HiddenExternal)
     return addFunctionToWorklist(F);
+  
+  // Update the linkage of the function in case it's different in the serialized
+  // SIL than derived from the AST. This can be the case with cross-module-
+  // optimizations.
+  Mod.updateFunctionLinkage(F);
 }
 
 /// Process F, recursively deserializing any thing F may reference.

lib/SIL/SILModule.cpp

@@ -321,14 +321,19 @@ SILFunction *SILModule::lookUpFunction(SILDeclRef fnRef) {
 }
 
 bool SILModule::loadFunction(SILFunction *F) {
-  SILFunction *NewF = getSILLoader()->lookupSILFunction(F);
+  SILFunction *NewF =
+    getSILLoader()->lookupSILFunction(F, /*onlyUpdateLinkage*/ false);
   if (!NewF)
     return false;
 
   assert(F == NewF);
   return true;
 }
 
+void SILModule::updateFunctionLinkage(SILFunction *F) {
+  getSILLoader()->lookupSILFunction(F, /*onlyUpdateLinkage*/ true);
+}
+
 bool SILModule::linkFunction(SILFunction *F, SILModule::LinkingMode Mode) {
   return SILLinkerVisitor(*this, Mode).processFunction(F);
 }

lib/Serialization/DeserializeSIL.cpp

@@ -527,12 +527,13 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
     replacedObjectiveCFunc = MF->getIdentifier(replacedFunctionID);
   }
 
-  auto linkage = fromStableSILLinkage(rawLinkage);
-  if (!linkage) {
+  auto linkageOpt = fromStableSILLinkage(rawLinkage);
+  if (!linkageOpt) {
     LLVM_DEBUG(llvm::dbgs() << "invalid linkage code " << rawLinkage
                             << " for SILFunction\n");
     MF->fatal();
   }
+  SILLinkage linkage = linkageOpt.getValue();
 
   ValueDecl *clangNodeOwner = nullptr;
   if (clangNodeOwnerID != 0) {
@@ -569,16 +570,22 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
     fn->setSerialized(IsSerialized_t(isSerialized));
 
     if (SILMod.getOptions().MergePartialModules)
-      fn->setLinkage(*linkage);
+      fn->setLinkage(linkage);
 
     // Don't override the transparency or linkage of a function with
     // an existing declaration, except if we deserialized a
     // PublicNonABI function, which has HiddenExternal when
     // referenced as a declaration, and SharedExternal when it has
     // a deserialized body.
-    if (isAvailableExternally(fn->getLinkage()) &&
-        linkage == SILLinkage::PublicNonABI) {
-      fn->setLinkage(SILLinkage::SharedExternal);
+    if (isAvailableExternally(fn->getLinkage())) {
+      if (linkage == SILLinkage::PublicNonABI) {
+        fn->setLinkage(SILLinkage::SharedExternal);
+      } else if (hasPublicVisibility(linkage)) {
+        // Cross-module-optimization can change the linkage to public. In this
+        // case we need to update the linkage of the function (which is
+        // originally just derived from the AST).
+        fn->setLinkage(SILLinkage::PublicExternal);
+      }
     }
 
     if (fn->isDynamicallyReplaceable() != isDynamic) {
@@ -589,7 +596,7 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
   } else {
     // Otherwise, create a new function.
     fn = builder.createDeclaration(name, ty, loc);
-    fn->setLinkage(linkage.getValue());
+    fn->setLinkage(linkage);
     fn->setTransparent(IsTransparent_t(isTransparent == 1));
     fn->setSerialized(IsSerialized_t(isSerialized));
     fn->setThunk(IsThunk_t(isThunk));
@@ -2545,16 +2552,18 @@ bool SILDeserializer::readSILInstruction(SILFunction *Fn, SILBasicBlock *BB,
   return false;
 }
 
-SILFunction *SILDeserializer::lookupSILFunction(SILFunction *InFunc) {
+SILFunction *SILDeserializer::lookupSILFunction(SILFunction *InFunc,
+                                                bool onlyUpdateLinkage) {
   StringRef name = InFunc->getName();
   if (!FuncTable)
     return nullptr;
   auto iter = FuncTable->find(name);
   if (iter == FuncTable->end())
     return nullptr;
 
+  // Re-reading the function as declaration will update the linkage.
   auto maybeFunc = readSILFunctionChecked(*iter, InFunc, name,
-                                          /*declarationOnly*/ false);
+                                        /*declarationOnly*/ onlyUpdateLinkage);
   if (!maybeFunc) {
     // Ignore the error; treat it as if we didn't have a definition.
     consumeError(maybeFunc.takeError());

lib/Serialization/DeserializeSIL.h

@@ -140,7 +140,7 @@ namespace swift {
     FileUnit *getFile() const {
       return MF->getFile();
     }
-    SILFunction *lookupSILFunction(SILFunction *InFunc);
+    SILFunction *lookupSILFunction(SILFunction *InFunc, bool onlyUpdateLinkage);
     SILFunction *lookupSILFunction(StringRef Name,
                                    bool declarationOnly = false);
     bool hasSILFunction(StringRef Name, Optional<SILLinkage> Linkage = None);

lib/Serialization/SerializedSILLoader.cpp

@@ -42,12 +42,13 @@ SerializedSILLoader::SerializedSILLoader(
 
 SerializedSILLoader::~SerializedSILLoader() {}
 
-SILFunction *SerializedSILLoader::lookupSILFunction(SILFunction *Callee) {
+SILFunction *SerializedSILLoader::lookupSILFunction(SILFunction *Callee,
+                                                    bool onlyUpdateLinkage) {
   // It is possible that one module has a declaration of a SILFunction, while
   // another has the full definition.
   SILFunction *retVal = nullptr;
   for (auto &Des : LoadedSILSections) {
-    if (auto Func = Des->lookupSILFunction(Callee)) {
+    if (auto Func = Des->lookupSILFunction(Callee, onlyUpdateLinkage)) {
       LLVM_DEBUG(llvm::dbgs() << "Deserialized " << Func->getName() << " from "
                  << Des->getModuleIdentifier().str() << "\n");
       if (!Func->empty())