Fixes for a lookup of functions by name

Among other things it fixes a bug in the function signature optimization where it would use an already existing mangled name for a different function

Author: swiftix

include/swift/SIL/SILModule.h

@@ -430,12 +430,19 @@ class SILModule {
   bool linkFunction(StringRef Name,
                     LinkingMode LinkAll = LinkingMode::LinkNormal);
 
-  /// Check if a given function exists in the module,
-  /// i.e. it can be linked by linkFunction.
+  /// Check if a given function exists in any of the modules with a
+  /// required linkage, i.e. it can be linked by linkFunction.
+  ///
+  /// If linkage parameter is none, then the linkage of the function
+  /// with a given name does not matter.
   ///
   /// \return null if this module has no such function. Otherwise
   /// the declaration of a function.
-  SILFunction *hasFunction(StringRef Name, SILLinkage Linkage);
+  SILFunction *findFunction(StringRef Name, Optional<SILLinkage> Linkage);
+
+  /// Check if a given function exists in the module,
+  /// i.e. it can be linked by linkFunction.
+  bool hasFunction(StringRef Name);
 
   /// Link in all Witness Tables in the module.
   void linkAllWitnessTables();

include/swift/Serialization/SerializedSILLoader.h

@@ -92,8 +92,8 @@ class SerializedSILLoader {
   SILFunction *lookupSILFunction(SILFunction *Callee);
   SILFunction *
   lookupSILFunction(StringRef Name, bool declarationOnly = false,
-                    SILLinkage linkage = SILLinkage::Private);
-  bool hasSILFunction(StringRef Name, SILLinkage linkage = SILLinkage::Private);
+                    Optional<SILLinkage> linkage = None);
+  bool hasSILFunction(StringRef Name, Optional<SILLinkage> linkage = None);
   SILVTable *lookupVTable(Identifier Name);
   SILVTable *lookupVTable(const ClassDecl *C) {
     return lookupVTable(C->getName());

lib/SIL/Linker.cpp

@@ -117,7 +117,8 @@ SILFunction *SILLinkerVisitor::lookupFunction(StringRef Name,
 }
 
 /// Process Decl, recursively deserializing any thing Decl may reference.
-bool SILLinkerVisitor::hasFunction(StringRef Name, SILLinkage Linkage) {
+bool SILLinkerVisitor::hasFunction(StringRef Name,
+                                   Optional<SILLinkage> Linkage) {
   return Loader->hasSILFunction(Name, Linkage);
 }
 

lib/SIL/Linker.h

@@ -60,7 +60,7 @@ class SILLinkerVisitor : public SILInstructionVisitor<SILLinkerVisitor, bool> {
 
   /// Process Name, try to check if there is a declaration of a function
   /// with this Name.
-  bool hasFunction(StringRef Name, SILLinkage Linkage);
+  bool hasFunction(StringRef Name, Optional<SILLinkage> Linkage = None);
 
   /// Deserialize the VTable mapped to C if it exists and all SIL the VTable
   /// transitively references.

lib/SIL/SILModule.cpp

@@ -499,23 +499,39 @@ bool SILModule::linkFunction(StringRef Name, SILModule::LinkingMode Mode) {
   return SILLinkerVisitor(*this, getSILLoader(), Mode).processFunction(Name);
 }
 
-SILFunction *SILModule::hasFunction(StringRef Name, SILLinkage Linkage) {
-  assert((Linkage == SILLinkage::Public ||
-          Linkage == SILLinkage::PublicExternal) &&
-         "Only a lookup of public functions is supported currently");
+/// Check if a given SIL linkage matches the required linkage.
+/// If the required linkage is Private, then anything matches it.
+static bool isMatchingLinkage(SILLinkage ActualLinkage,
+                              Optional<SILLinkage> Linkage) {
+  if (!Linkage)
+    return true;
+  return ActualLinkage == Linkage;
+}
+
+bool SILModule::hasFunction(StringRef Name) {
+  if (lookUpFunction(Name))
+    return true;
+  SILLinkerVisitor Visitor(*this, getSILLoader(),
+                           SILModule::LinkingMode::LinkNormal);
+  return Visitor.hasFunction(Name);
+}
+
+SILFunction *SILModule::findFunction(StringRef Name,
+                                     Optional<SILLinkage> Linkage) {
 
   SILFunction *F = nullptr;
+  SILLinkage RequiredLinkage = Linkage ? *Linkage : SILLinkage::Private;
 
   // First, check if there is a function with a required name in the
   // current module.
   SILFunction *CurF = lookUpFunction(Name);
 
   // Nothing to do if the current module has a required function
   // with a proper linkage already.
-  if (CurF && CurF->getLinkage() == Linkage) {
+  if (CurF && isMatchingLinkage(CurF->getLinkage(), Linkage)) {
     F = CurF;
   } else {
-    assert((!CurF || CurF->getLinkage() != Linkage) &&
+    assert((!CurF || CurF->getLinkage() != RequiredLinkage) &&
            "hasFunction should be only called for functions that are not "
            "contained in the SILModule yet or do not have a required linkage");
   }
@@ -528,7 +544,7 @@ SILFunction *SILModule::hasFunction(StringRef Name, SILLinkage Linkage) {
       // name is present in the current module.
       // This is done to reduce the amount of IO from the
       // swift module file.
-      if (!Visitor.hasFunction(Name, Linkage))
+      if (!Visitor.hasFunction(Name, RequiredLinkage))
         return nullptr;
       // The function in the current module will be changed.
       F = CurF;
@@ -538,13 +554,14 @@ SILFunction *SILModule::hasFunction(StringRef Name, SILLinkage Linkage) {
     // or if it is known to exist, perform a lookup.
     if (!F) {
       // Try to load the function from other modules.
-      F = Visitor.lookupFunction(Name, Linkage);
+      F = Visitor.lookupFunction(Name, RequiredLinkage);
       // Bail if nothing was found and we are not sure if
       // this function exists elsewhere.
       if (!F)
         return nullptr;
       assert(F && "SILFunction should be present in one of the modules");
-      assert(F->getLinkage() == Linkage && "SILFunction has a wrong linkage");
+      assert(isMatchingLinkage(F->getLinkage(), Linkage) &&
+             "SILFunction has a wrong linkage");
     }
   }
 
@@ -556,9 +573,19 @@ SILFunction *SILModule::hasFunction(StringRef Name, SILLinkage Linkage) {
           SILOptions::SILOptMode::Optimize) {
     F->convertToDeclaration();
   }
-  if (F->isExternalDeclaration())
+  if (F->isExternalDeclaration()) {
     F->setFragile(IsFragile_t::IsNotFragile);
-  F->setLinkage(Linkage);
+    if (isAvailableExternally(F->getLinkage()) &&
+        !hasPublicVisibility(F->getLinkage()) && !Linkage) {
+      // We were just asked if a given function exists anywhere.
+      // It is not going to be used by the current module.
+      // Since external non-public function declarations should not
+      // exist, strip the "external" part from the linkage.
+      F->setLinkage(stripExternalFromLinkage(F->getLinkage()));
+    }
+  }
+  if (Linkage)
+    F->setLinkage(RequiredLinkage);
   return F;
 }
 

lib/SILOptimizer/IPO/EagerSpecializer.cpp

@@ -287,7 +287,7 @@ class EagerDispatch {
         substConv(ReInfo.getSubstitutedType(), GenericFunc->getModule()),
         Builder(*GenericFunc), Loc(GenericFunc->getLocation()) {
     Builder.setCurrentDebugScope(GenericFunc->getDebugScope());
-    IsClassF = Builder.getModule().hasFunction(
+    IsClassF = Builder.getModule().findFunction(
       "_swift_isClassOrObjCExistentialType", SILLinkage::PublicExternal);
     assert(IsClassF);
   }

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

@@ -93,7 +93,7 @@ static SILInstruction *findOnlyApply(SILFunction *F) {
 ///
 /// TODO: we should teach the demangler to understand this suffix.
 static std::string getUniqueName(std::string Name, SILModule &M) {
-  if (!M.lookUpFunction(Name))
+  if (!M.hasFunction(Name))
     return Name;
   return getUniqueName(Name + "_unique_suffix", M);
 }
@@ -372,7 +372,7 @@ std::string FunctionSignatureTransform::createOptimizedSILFunctionName() {
   do {
     New = NewFM.mangle(UniqueID);
     ++UniqueID;
-  } while (M.lookUpFunction(New));
+  } while (M.hasFunction(New));
 
   return NewMangling::selectMangling(Old, New);
 }

lib/SILOptimizer/Utils/Generics.cpp

@@ -1180,7 +1180,7 @@ static SILFunction *lookupExistingSpecialization(SILModule &M,
   // Only check that this function exists, but don't read
   // its body. It can save some compile-time.
   if (isWhitelistedSpecialization(FunctionName))
-    return M.hasFunction(FunctionName, SILLinkage::PublicExternal);
+    return M.findFunction(FunctionName, SILLinkage::PublicExternal);
 
   return nullptr;
 }

lib/Serialization/DeserializeSIL.cpp

@@ -1993,7 +1993,7 @@ SILFunction *SILDeserializer::lookupSILFunction(SILFunction *InFunc) {
 /// This function is modeled after readSILFunction. But it does not
 /// create a SILFunction object.
 bool SILDeserializer::hasSILFunction(StringRef Name,
-                                     SILLinkage Linkage) {
+                                     Optional<SILLinkage> Linkage) {
   if (!FuncTable)
     return false;
   auto iter = FuncTable->find(Name);
@@ -2006,8 +2006,7 @@ bool SILDeserializer::hasSILFunction(StringRef Name,
   auto &cacheEntry = Funcs[FID-1];
   if (cacheEntry.isFullyDeserialized() ||
       (cacheEntry.isDeserialized()))
-    return cacheEntry.get()->getLinkage() == Linkage ||
-           Linkage == SILLinkage::Private;
+    return !Linkage || cacheEntry.get()->getLinkage() == *Linkage;
 
   BCOffsetRAII restoreOffset(SILCursor);
   SILCursor.JumpToBit(cacheEntry.getOffset());
@@ -2046,7 +2045,7 @@ bool SILDeserializer::hasSILFunction(StringRef Name,
   }
 
   // Bail if it is not a required linkage.
-  if (linkage.getValue() != Linkage && Linkage != SILLinkage::Private)
+  if (Linkage && linkage.getValue() != *Linkage)
     return false;
 
   DEBUG(llvm::dbgs() << "Found SIL Function: " << Name << "\n");

lib/Serialization/DeserializeSIL.h

@@ -118,7 +118,7 @@ namespace swift {
     SILFunction *lookupSILFunction(SILFunction *InFunc);
     SILFunction *lookupSILFunction(StringRef Name,
                                    bool declarationOnly = false);
-    bool hasSILFunction(StringRef Name, SILLinkage Linkage);
+    bool hasSILFunction(StringRef Name, Optional<SILLinkage> Linkage = None);
     SILVTable *lookupVTable(Identifier Name);
     SILWitnessTable *lookupWitnessTable(SILWitnessTable *wt);
     SILDefaultWitnessTable *

lib/Serialization/SerializedSILLoader.cpp

@@ -59,17 +59,17 @@ SILFunction *SerializedSILLoader::lookupSILFunction(SILFunction *Callee) {
 
 SILFunction *SerializedSILLoader::lookupSILFunction(StringRef Name,
                                                     bool declarationOnly,
-                                                    SILLinkage Linkage) {
+                                                    Optional<SILLinkage> Linkage) {
   // 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(Name, declarationOnly)) {
       DEBUG(llvm::dbgs() << "Deserialized " << Func->getName() << " from "
             << Des->getModuleIdentifier().str() << "\n");
-      if (Linkage != SILLinkage::Private) {
+      if (Linkage) {
         // This is not the linkage we are looking for.
-        if (Func->getLinkage() != Linkage) {
+        if (Func->getLinkage() != *Linkage) {
           DEBUG(llvm::dbgs()
                 << "Wrong linkage for Function: " << Func->getName() << " : "
                 << (int)Func->getLinkage() << "\n");
@@ -86,7 +86,8 @@ SILFunction *SerializedSILLoader::lookupSILFunction(StringRef Name,
   return retVal;
 }
 
-bool SerializedSILLoader::hasSILFunction(StringRef Name, SILLinkage Linkage) {
+bool SerializedSILLoader::hasSILFunction(StringRef Name,
+                                         Optional<SILLinkage> Linkage) {
   // It is possible that one module has a declaration of a SILFunction, while
   // another has the full definition.
   SILFunction *retVal = nullptr;