Merge pull request #28559 from eeckstein/fix-cmo

Cross-module-optimization: no need to add AST-attributes to make functions always-emit-into-client.

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/SIL/SILPrinter.cpp

@@ -222,9 +222,10 @@ static void printFullContext(const DeclContext *Context, raw_ostream &Buffer) {
 
 static void printValueDecl(ValueDecl *Decl, raw_ostream &OS) {
   printFullContext(Decl->getDeclContext(), OS);
-  assert(Decl->hasName());
 
-  if (Decl->isOperator()) {
+  if (!Decl->hasName()) {
+    OS << "anonname=" << (const void*)Decl;
+  } else if (Decl->isOperator()) {
     OS << '"' << Decl->getBaseName() << '"';
   } else {
     bool shouldEscape = !Decl->getBaseName().isSpecial() &&

lib/SILOptimizer/IPO/CrossModuleSerializationSetup.cpp

@@ -61,8 +61,6 @@ class CrossModuleSerializationSetup {
 
   void makeSubstUsableFromInline(const SubstitutionMap &substs);
 
-  bool markAsEmitIntoClient(SILFunction *F);
-
 public:
   CrossModuleSerializationSetup(SILModule &M) : M(M) { }
 
@@ -313,12 +311,7 @@ void CrossModuleSerializationSetup::scanModule() {
 
         // As a code size optimization, make serialized functions
         // @alwaysEmitIntoClient.
-        if (!markAsEmitIntoClient(F)) {
-          // We don't have a declaration to put the attribute on (e.g. in case
-          // the function is a closure). Just make the function public instead
-          // of @alwaysEmitIntoClient.
-          F->setLinkage(SILLinkage::Public);
-        }
+        F->setLinkage(SILLinkage::PublicNonABI);
       } else {
         // If for some reason the function cannot be serialized, we mark it as
         // usable-from-inline.
@@ -328,36 +321,6 @@ void CrossModuleSerializationSetup::scanModule() {
   }
 }
 
-/// Marks a function as @alwaysEmitIntoClient and returns true if this is
-/// successful.
-bool CrossModuleSerializationSetup::markAsEmitIntoClient(SILFunction *F) {
-  auto *DC = F->getDeclContext();
-  if (!DC)
-    return false;
-
-  Decl *decl = DC->getAsDecl();
-  if (!decl)
-    return false;
-
-  if (!isa<AbstractFunctionDecl>(decl))
-    return false;
-
-  F->setLinkage(SILLinkage::PublicNonABI);
-
-  // Adding the attribute is only needed to be able to compile the
-  // client module with -Onone. For optimized builds, setting the
-  // SILLinkage is enough, because with optimization, the client module
-  // eagerly de-serializes all functions and therefore gets the
-  // linkage right. But with -Onone, the linkage is derived purly from
-  // the AST.
-  // TODO: also here, we should find a way to not modify the AST.
-  auto &ctx = M.getASTContext();
-  auto *attr = new (ctx) AlwaysEmitIntoClientAttr(/*implicit=*/true);
-  decl->getAttrs().add(attr);
-
-  return true;
-}
-
 class CrossModuleSerializationSetupPass: public SILModuleTransform {
   void run() override {
 

lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp

@@ -618,6 +618,8 @@ namespace {
         Changed |= tryToSpeculateTarget(AI, CHA, ORE);
 
       if (Changed) {
+        CurFn.getModule().linkFunction(&CurFn, SILModule::LinkingMode::LinkAll);
+
         invalidateAnalysis(SILAnalysis::InvalidationKind::FunctionBody);
       }
     }

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())

test/SILOptimizer/Inputs/cross-module.swift

@@ -92,6 +92,11 @@ private protocol PrivateProtocol {
 
 open class OpenClass<T> {
   public init() { }
+
+  @inline(never)
+  fileprivate func bar(_ t: T) {
+    print(t)
+  }
 }
 
 extension OpenClass {
@@ -114,6 +119,12 @@ public func checkIfClassConforms_gen<T>(_ t: T) {
   print(x.testit())
 }
 
+@inline(never)
+public func callClassMethod<T>(_ t: T) {
+  let k = OpenClass<T>()
+  k.bar(t)
+}
+
 extension Int : PrivateProtocol {
   func foo() -> Int { return self }
 }

test/SILOptimizer/cross-module-optimization.swift

@@ -64,7 +64,9 @@ func testError() {
   print(returnInternalError(27))
 }
 
-func testProtocol() {
+class DerivedFromOpen<T> : OpenClass<T> { }
+
+func testProtocolsAndClasses() {
   // CHECK-OUTPUT: false
   // CHECK-SIL-DAG: sil shared [noinline] @$s4Test20checkIfClassConformsyyxlFSi_Tg5
   checkIfClassConforms(27)
@@ -80,6 +82,8 @@ func testProtocol() {
   // CHECK-OUTPUT: 1234
   // CHECK-SIL-DAG: sil shared_external {{.*}} @$s4Test11callFoo_genyyxlF
   callFoo_gen(27)
+  // CHECK-OUTPUT: 55
+  callClassMethod(55)
 }
 
 func testSubModule() {
@@ -111,7 +115,7 @@ func testKeypath() {
 testNestedTypes()
 testClass()
 testError()
-testProtocol()
+testProtocolsAndClasses()
 testSubModule()
 testClosures()
 testKeypath()