Richard's suggestion about having a separate function instead of 
HandleTopLevelDecl that would be used for updating the linkage after explicit 
instantiation got me thinking about how that would work if the definition had 
already been emitted.

To be able to change the linkage from weak to strong, it would be nice if the 
function was not emitted yet. This is what happens with normal implicit 
template instantiations today: on first use (which is typically what triggered 
the instantiation), they get moved into deferredDeclsToEmit, and then stay 
there until the end of the TU.

The only reason this doesn't work with dllexport or attribute(used) functions 
is that MayDeferGeneration() returns false for those. That's actually a 
misnomer because deferring is not a problem for them, as long as they are 
emitted in the end.

This patch tries to solve the problem by reorganizing the logic about deferring 
generation.

At least this feels a lot less hacky than the previous patch :)


http://reviews.llvm.org/D6674

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/dllexport.cpp
  test/CodeGenCXX/explicit-instantiation.cpp

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1102,14 +1102,18 @@
     llvm::GlobalValue *GV = G.GV;
     DeferredDeclsToEmit.pop_back();
 
-    assert(GV == GetGlobalValue(getMangledName(D)));
+    if (GV)
+      assert(GV == GetGlobalValue(getMangledName(D)));
+    else
+      GV = GetGlobalValue(getMangledName(D));
+
     // Check to see if we've already emitted this.  This is necessary
     // for a couple of reasons: first, decls can end up in the
     // deferred-decls queue multiple times, and second, decls can end
     // up with definitions in unusual ways (e.g. by an extern inline
     // function acquiring a strong function redefinition).  Just
     // ignore these cases.
-    if(!GV->isDeclaration())
+    if (GV && !GV->isDeclaration())
       continue;
 
     // Otherwise, emit the definition and move on to the next one.
@@ -1234,12 +1238,22 @@
   return false;
 }
 
-bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
+bool CodeGenModule::MustBeEmitted(const ValueDecl *Global) {
   // Never defer when EmitAllDecls is specified.
   if (LangOpts.EmitAllDecls)
-    return false;
+    return true;
+
+  return getContext().DeclMustBeEmitted(Global);
+}
 
-  return !getContext().DeclMustBeEmitted(Global);
+bool CodeGenModule::MayBeEmittedEagerly(const ValueDecl *Global) {
+  if (const auto *FD = dyn_cast<FunctionDecl>(Global))
+    if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+      // Implicit template instantiations may change linkage if they are later
+      // explicitly instantiated, so they should not be emitted eagerly.
+      return false;
+
+  return true;
 }
 
 llvm::Constant *CodeGenModule::GetAddrOfUuidDescriptor(
@@ -1348,9 +1362,10 @@
       return;
   }
 
-  // Defer code generation when possible if this is a static definition, inline
-  // function etc.  These we only want to emit if they are used.
-  if (!MayDeferGeneration(Global)) {
+  // Defer code generation to first use when possible, e.g. if this is an inline
+  // function. If the global must always be emitted, do it eagerly if possible
+  // to benefit from cache locality.
+  if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
     // Emit the definition if it can't be deferred.
     EmitGlobalDefinition(GD);
     return;
@@ -1363,13 +1378,16 @@
     DelayedCXXInitPosition[Global] = CXXGlobalInits.size();
     CXXGlobalInits.push_back(nullptr);
   }
-  
-  // If the value has already been used, add it directly to the
-  // DeferredDeclsToEmit list.
+
   StringRef MangledName = getMangledName(GD);
-  if (llvm::GlobalValue *GV = GetGlobalValue(MangledName))
+  if (llvm::GlobalValue *GV = GetGlobalValue(MangledName)) {
+    // The value has already been used and should therefore be emitted.
     addDeferredDeclToEmit(GV, GD);
-  else {
+  } else if (MustBeEmitted(Global)) {
+    // The value must be emitted, but cannot be emitted eagerly.
+    assert(!MayBeEmittedEagerly(Global));
+    addDeferredDeclToEmit(nullptr, GD);
+  } else {
     // Otherwise, remember that we saw a deferred decl with this name.  The
     // first use of the mangled name will cause it to move into
     // DeferredDeclsToEmit.
@@ -1843,7 +1861,7 @@
 void CodeGenModule::EmitTentativeDefinition(const VarDecl *D) {
   assert(!D->getInit() && "Cannot emit definite definitions here!");
 
-  if (MayDeferGeneration(D)) {
+  if (!MustBeEmitted(D)) {
     // If we have not seen a reference to this variable yet, place it
     // into the deferred declarations table to be emitted if needed
     // later.
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1195,9 +1195,15 @@
   /// Emits the initializer for a uuidof string.
   llvm::Constant *EmitUuidofInitializer(StringRef uuidstr);
 
-  /// Determine if the given decl can be emitted lazily; this is only relevant
-  /// for definitions. The given decl must be either a function or var decl.
-  bool MayDeferGeneration(const ValueDecl *D);
+  /// Determine if the given definition must be emitted, or can otherwise be
+  /// emitted lazily.
+  bool MustBeEmitted(const ValueDecl *D);
+
+  /// Determine whether the definition can be emitted eagerly, or should be
+  /// delayed until the end of the translation unit. This is relevant for
+  /// definitions whose linkage can change, e.g. implicit function instantions
+  /// which may later be explicitly instantiated.
+  bool MayBeEmittedEagerly(const ValueDecl *D);
 
   /// Check whether we can use a "simpler", more core exceptions personality
   /// function.
Index: test/CodeGenCXX/dllexport.cpp
===================================================================
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -615,6 +615,21 @@
 struct __declspec(dllexport) ExportedDerivedClass : NonExportedBaseClass {};
 // M32-DAG: weak_odr dllexport x86_thiscallcc void @"\01??1ExportedDerivedClass@@UAE@XZ"
 
+// Do not assert about generating code for constexpr functions twice during explicit instantiation (PR21718).
+template <typename T> struct ExplicitInstConstexprMembers {
+  // Copy assignment operator
+  // M32-DAG: define weak_odr dllexport x86_thiscallcc dereferenceable(1) %struct.ExplicitInstConstexprMembers* @"\01??4?$ExplicitInstConstexprMembers@X@@QAEAAU0@ABU0@@Z"
+
+  constexpr ExplicitInstConstexprMembers() {}
+  // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.ExplicitInstConstexprMembers* @"\01??0?$ExplicitInstConstexprMembers@X@@QAE@XZ"
+
+  ExplicitInstConstexprMembers(const ExplicitInstConstexprMembers&) = default;
+  // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.ExplicitInstConstexprMembers* @"\01??0?$ExplicitInstConstexprMembers@X@@QAE@ABU0@@Z"
+
+  constexpr int f() const { return 42; }
+  // M32-DAG: define weak_odr dllexport x86_thiscallcc i32 @"\01?f@?$ExplicitInstConstexprMembers@X@@QBEHXZ"
+};
+template struct __declspec(dllexport) ExplicitInstConstexprMembers<void>;
 
 //===----------------------------------------------------------------------===//
 // Classes with template base classes
Index: test/CodeGenCXX/explicit-instantiation.cpp
===================================================================
--- test/CodeGenCXX/explicit-instantiation.cpp
+++ test/CodeGenCXX/explicit-instantiation.cpp
@@ -90,6 +90,19 @@
   // CHECK-OPT: define available_externally i32 @_ZN17LateInstantiation1fIiEEiv(
 }
 
+namespace PR21718 {
+// The linkage of a used constexpr member function can change from linkonce_odr
+// to weak_odr after explicit instantiation without errors about defining the
+// same function twice.
+template <typename T>
+struct S {
+// CHECK-LABEL: define weak_odr i32 @_ZN7PR217181SIiE1fEv
+  __attribute__((used)) constexpr int f() { return 0; }
+};
+int g() { return S<int>().f(); }
+template struct S<int>;
+}
+
 // Check that we emit definitions from explicit instantiations even when they
 // occur prior to the definition itself.
 template <typename T> struct S {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to