dblaikie created this revision.

First pass at generating weak definitions of inline functions from module files
(& currently skipping definitions of those functions in uses)

I've done some manual testing (haven't delved into the preferred testing
strategy for modules). Seems to work for simplest cases, including transitive
modules.

Also doesn't have an actual flag plumbed through, yet. I can submit that ahead
of this in a separate patch (open to ideas, etc). Is there a goal for how
explicit modules should be used via the driver (rather than cc1)? Otherwise
this would be a cc1 option to match.

One quandry: What to do with non-inline definitions in headers? Currently this
prototype moves their emission (while keeping their linkage) to the module
object. This would remove ODR violations and actually make it 'correct' to
define a non-inline function in a header, which may or may not be desirable. It
could be kept in all the users as it is today.

I'm sure I don't know nearly enough about linkage and about how linkage is
handled in Clang & LLVM. So I'm certainly open to ideas about how this
implementation isn't ideal or correct - or the myriad of interesting linkage
cases I should test/consider.

Perhaps we could chat about this in person/over the shoulder if a faster
iteration would be worthwhile.


https://reviews.llvm.org/D28845

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/ExternalASTSource.h
  include/clang/Basic/LangOptions.def
  include/clang/Basic/Module.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ASTWriter.h
  lib/AST/ASTContext.cpp
  lib/AST/ExternalASTSource.cpp
  lib/Basic/Module.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Lex/ModuleMap.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp

Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -2152,7 +2152,7 @@
 /// relatively painless since they would presumably only do it for top-level
 /// decls.
 static bool isRequiredDecl(const Decl *D, ASTContext &Context,
-                           bool WritingModule) {
+                           bool WritingModule, bool ModularCode) {
   // An ObjCMethodDecl is never considered as "required" because its
   // implementation container always is.
 
@@ -2168,7 +2168,7 @@
     return false;
   }
 
-  return Context.DeclMustBeEmitted(D);
+  return Context.DeclMustBeEmitted(D, ModularCode);
 }
 
 void ASTWriter::WriteDecl(ASTContext &Context, Decl *D) {
@@ -2212,8 +2212,11 @@
 
   // Note declarations that should be deserialized eagerly so that we can add
   // them to a record in the AST file later.
-  if (isRequiredDecl(D, Context, WritingModule))
+  if (isRequiredDecl(D, Context, WritingModule, false))
     EagerlyDeserializedDecls.push_back(ID);
+  else if (Context.getLangOpts().ModularCodegen && WritingModule &&
+           isRequiredDecl(D, Context, true, true))
+    ModularCodegenDecls.push_back(ID);
 }
 
 void ASTRecordWriter::AddFunctionDefinition(const FunctionDecl *FD) {
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1030,6 +1030,7 @@
   RECORD(IDENTIFIER_OFFSET);
   RECORD(IDENTIFIER_TABLE);
   RECORD(EAGERLY_DESERIALIZED_DECLS);
+  RECORD(MODULAR_CODEGEN_DECLS);
   RECORD(SPECIAL_TYPES);
   RECORD(STATISTICS);
   RECORD(TENTATIVE_DEFINITIONS);
@@ -4675,6 +4676,9 @@
   if (!EagerlyDeserializedDecls.empty())
     Stream.EmitRecord(EAGERLY_DESERIALIZED_DECLS, EagerlyDeserializedDecls);
 
+  if (Context.getLangOpts().ModularCodegen)
+    Stream.EmitRecord(MODULAR_CODEGEN_DECLS, ModularCodegenDecls);
+
   // Write the record containing tentative definitions.
   if (!TentativeDefinitions.empty())
     Stream.EmitRecord(TENTATIVE_DEFINITIONS, TentativeDefinitions);
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2533,6 +2533,9 @@
     return Failure;
   }
 
+  Module *Root = nullptr;
+  Module::ExtKind EK = Module::EK_ReplyHazy;
+
   // Read all of the records and blocks for the AST file.
   RecordData Record;
   while (true) {
@@ -2553,6 +2556,8 @@
           !getContext().getLangOpts().CPlusPlus)
         DC->setMustBuildLookupTable();
 
+      if (Root)
+        Root->setExternalDefinitions(EK);
       return Success;
     }
     case llvm::BitstreamEntry::SubBlock:
@@ -2607,7 +2612,8 @@
         break;
 
       case SUBMODULE_BLOCK_ID:
-        if (ASTReadResult Result = ReadSubmoduleBlock(F, ClientLoadCapabilities))
+        if (ASTReadResult Result =
+                ReadSubmoduleBlock(F, ClientLoadCapabilities, Root))
           return Result;
         break;
 
@@ -2772,6 +2778,15 @@
         EagerlyDeserializedDecls.push_back(getGlobalDeclID(F, Record[I]));
       break;
 
+    case MODULAR_CODEGEN_DECLS:
+      EK = F.Kind == MK_MainFile ? Module::EK_Never : Module::EK_Always;
+      // FIXME: Skip reading this record if our ASTConsumer doesn't care about
+      // them (ie: if we're not codegenerating this module).
+      if (F.Kind == MK_MainFile)
+        for (unsigned I = 0, N = Record.size(); I != N; ++I)
+          EagerlyDeserializedDecls.push_back(getGlobalDeclID(F, Record[I]));
+      break;
+
     case SPECIAL_TYPES:
       if (SpecialTypes.empty()) {
         for (unsigned I = 0, N = Record.size(); I != N; ++I)
@@ -4562,7 +4577,8 @@
 }
 
 ASTReader::ASTReadResult
-ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
+ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities,
+                              Module *&Root) {
   // Enter the submodule block.
   if (F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) {
     Error("malformed submodule block record in AST file");
@@ -4634,8 +4650,12 @@
 
       // Retrieve this (sub)module from the module map, creating it if
       // necessary.
-      CurrentModule = ModMap.findOrCreateModule(Name, ParentModule, IsFramework,
-                                                IsExplicit).first;
+      CurrentModule =
+          ModMap.findOrCreateModule(Name, ParentModule, IsFramework, IsExplicit)
+              .first;
+
+      if (!Parent)
+        Root = CurrentModule;
 
       // FIXME: set the definition loc for CurrentModule, or call
       // ModMap.setInferredModuleAllowedBy()
@@ -7849,6 +7869,13 @@
   return None;
 }
 
+Module::ExtKind ASTReader::hasExternalDefinitions(unsigned ID) {
+  if (const Module *M = getSubmodule(ID))
+    return M->hasExternalDefinitions();
+
+  return Module::EK_ReplyHazy;
+}
+
 Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
   return DecodeSelector(getGlobalSelectorID(M, LocalID));
 }
Index: lib/Lex/ModuleMap.cpp
===================================================================
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -554,16 +554,17 @@
   return Context->findSubmodule(Name);
 }
 
-std::pair<Module *, bool> 
-ModuleMap::findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
-                              bool IsExplicit) {
+std::pair<Module *, bool> ModuleMap::findOrCreateModule(StringRef Name,
+                                                        Module *Parent,
+                                                        bool IsFramework,
+                                                        bool IsExplicit) {
   // Try to find an existing module with this name.
   if (Module *Sub = lookupModuleQualified(Name, Parent))
     return std::make_pair(Sub, false);
   
   // Create a new module with this name.
-  Module *Result = new Module(Name, SourceLocation(), Parent,
-                              IsFramework, IsExplicit, NumCreatedModules++);
+  Module *Result = new Module(Name, SourceLocation(), Parent, IsFramework,
+                              IsExplicit, NumCreatedModules++);
   if (!Parent) {
     if (LangOpts.CurrentModule == Name)
       SourceModule = Result;
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2800,10 +2800,22 @@
       return llvm::GlobalVariable::WeakAnyLinkage;
   }
 
+  if (auto *Ext = getContext().getExternalSource())
+    switch (Ext->hasExternalDefinitions(D->getOwningModuleID())) {
+    case Module::EK_Never:
+      if (Linkage == GVA_DiscardableODR)
+        return llvm::GlobalValue::WeakODRLinkage;
+      break;
+    case Module::EK_Always:
+      return llvm::GlobalValue::AvailableExternallyLinkage;
+    case Module::EK_ReplyHazy:
+      break;
+    }
+
   // We are guaranteed to have a strong definition somewhere else,
   // so we can use available_externally linkage.
   if (Linkage == GVA_AvailableExternally)
-    return llvm::Function::AvailableExternallyLinkage;
+    return llvm::GlobalValue::AvailableExternallyLinkage;
 
   // Note that Apple's kernel linker doesn't support symbol
   // coalescing, so we need to avoid linkonce and weak linkages there.
Index: lib/Basic/Module.cpp
===================================================================
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -33,7 +33,8 @@
       IsExplicit(IsExplicit), IsSystem(false), IsExternC(false),
       IsInferred(false), InferSubmodules(false), InferExplicitSubmodules(false),
       InferExportWildcard(false), ConfigMacrosExhaustive(false),
-      NoUndeclaredIncludes(false), NameVisibility(Hidden) {
+      NoUndeclaredIncludes(false), ExtDef(EK_ReplyHazy),
+      NameVisibility(Hidden) {
   if (Parent) {
     if (!Parent->isAvailable())
       IsAvailable = false;
Index: lib/AST/ExternalASTSource.cpp
===================================================================
--- lib/AST/ExternalASTSource.cpp
+++ lib/AST/ExternalASTSource.cpp
@@ -28,6 +28,10 @@
   return None;
 }
 
+Module::ExtKind ExternalASTSource::hasExternalDefinitions(unsigned ID) {
+  return Module::EK_ReplyHazy;
+}
+
 ExternalASTSource::ASTSourceDescriptor::ASTSourceDescriptor(const Module &M)
   : Signature(M.Signature), ClangModule(&M) {
   if (M.Directory)
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -8938,7 +8938,7 @@
       *this, basicGVALinkageForVariable(*this, VD), VD);
 }
 
-bool ASTContext::DeclMustBeEmitted(const Decl *D) {
+bool ASTContext::DeclMustBeEmitted(const Decl *D, bool WritingModule) {
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
     if (!VD->isFileVarDecl())
       return false;
@@ -9002,6 +9002,17 @@
       }
     }
 
+    GVALinkage Linkage = GetGVALinkageForFunction(FD);
+
+    if (Linkage == GVA_DiscardableODR) {
+      if (WritingModule)
+        return true;
+      if (auto *Ext = getExternalSource())
+        if (Ext->hasExternalDefinitions(FD->getOwningModuleID()) ==
+            Module::EK_Never)
+          return true;
+    }
+
     // static, static inline, always_inline, and extern inline functions can
     // always be deferred.  Normal inline functions can be deferred in C99/C++.
     // Implicit template instantiations can also be deferred in C++.
Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -365,6 +365,7 @@
   /// IDs, since they will be written out to an EAGERLY_DESERIALIZED_DECLS
   /// record.
   SmallVector<uint64_t, 16> EagerlyDeserializedDecls;
+  SmallVector<uint64_t, 16> ModularCodegenDecls;
 
   /// \brief DeclContexts that have received extensions since their serialized
   /// form.
Index: include/clang/Serialization/ASTReader.h
===================================================================
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -715,6 +715,8 @@
   /// the consumer eagerly.
   SmallVector<uint64_t, 16> EagerlyDeserializedDecls;
 
+  SmallVector<uint64_t, 16> ModularCodegenDecls;
+
   /// \brief The IDs of all tentative definitions stored in the chain.
   ///
   /// Sema keeps track of all tentative definitions in a TU because it has to
@@ -1194,7 +1196,8 @@
                                        const ModuleFile *ImportedBy,
                                        unsigned ClientLoadCapabilities);
   ASTReadResult ReadSubmoduleBlock(ModuleFile &F,
-                                   unsigned ClientLoadCapabilities);
+                                   unsigned ClientLoadCapabilities,
+                                   Module *&Root);
   static bool ParseLanguageOptions(const RecordData &Record, bool Complain,
                                    ASTReaderListener &Listener,
                                    bool AllowCompatibleDifferences);
@@ -1968,6 +1971,8 @@
   /// \brief Return a descriptor for the corresponding module.
   llvm::Optional<ASTSourceDescriptor> getSourceDescriptor(unsigned ID) override;
 
+  Module::ExtKind hasExternalDefinitions(unsigned ID) override;
+
   /// \brief Retrieve a selector from the given module with its local ID
   /// number.
   Selector getLocalSelector(ModuleFile &M, unsigned LocalID);
Index: include/clang/Serialization/ASTBitCodes.h
===================================================================
--- include/clang/Serialization/ASTBitCodes.h
+++ include/clang/Serialization/ASTBitCodes.h
@@ -591,6 +591,8 @@
 
       /// \brief Record code for declarations associated with OpenCL extensions.
       OPENCL_EXTENSION_DECLS = 59,
+
+      MODULAR_CODEGEN_DECLS = 60,
     };
 
     /// \brief Record types used within a source manager block.
Index: include/clang/Basic/Module.h
===================================================================
--- include/clang/Basic/Module.h
+++ include/clang/Basic/Module.h
@@ -205,6 +205,8 @@
   /// and headers from used modules.
   unsigned NoUndeclaredIncludes : 1;
 
+  unsigned ExtDef : 2;
+
   /// \brief Describes the visibility of the various names within a
   /// particular module.
   enum NameVisibilityKind {
@@ -307,12 +309,26 @@
   /// \brief The list of conflicts.
   std::vector<Conflict> Conflicts;
 
+  enum ExtKind { EK_Always, EK_Never, EK_ReplyHazy };
+
   /// \brief Construct a new module or submodule.
   Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
          bool IsFramework, bool IsExplicit, unsigned VisibilityID);
-  
+
   ~Module();
-  
+
+  void setExternalDefinitions(ExtKind EK) {
+    assert(!Parent);
+    ExtDef = EK;
+  }
+
+  ExtKind hasExternalDefinitions() const {
+    auto *P = this;
+    while (auto *K = P->Parent)
+      P = K;
+    return static_cast<ExtKind>(P->ExtDef);
+  }
+
   /// \brief Determine whether this module is available for use within the
   /// current translation unit.
   bool isAvailable() const { return IsAvailable; }
Index: include/clang/Basic/LangOptions.def
===================================================================
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -201,6 +201,7 @@
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(NewAlignOverride  , 32, 0, "maximum alignment guaranteed by '::operator new(size_t)'")
 LANGOPT(ConceptsTS , 1, 0, "enable C++ Extensions for Concepts")
+BENIGN_LANGOPT(ModularCodegen , 1, 1, "Modular codegen")
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd records in a simple form")
Index: include/clang/AST/ExternalASTSource.h
===================================================================
--- include/clang/AST/ExternalASTSource.h
+++ include/clang/AST/ExternalASTSource.h
@@ -16,6 +16,7 @@
 
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/Module.h"
 #include "llvm/ADT/DenseMap.h"
 
 namespace clang {
@@ -169,6 +170,8 @@
   /// Return a descriptor for the corresponding module, if one exists.
   virtual llvm::Optional<ASTSourceDescriptor> getSourceDescriptor(unsigned ID);
 
+  virtual Module::ExtKind hasExternalDefinitions(unsigned ID);
+
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.
   ///
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2487,7 +2487,7 @@
   ///
   /// \returns true if the function/var must be CodeGen'ed/deserialized even if
   /// it is not used.
-  bool DeclMustBeEmitted(const Decl *D);
+  bool DeclMustBeEmitted(const Decl *D, bool WritingModule = false);
 
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to