Rework when and how vtables are emitted, by tracking where vtables are

"used" (e.g., we will refer to the vtable in the generated code) and
when they are defined (i.e., because we've seen the key function
definition). Previously, we were effectively tracking "potential
definitions" rather than uses, so we were a bit too eager about emitting
vtables for classes without key functions. 

The new scheme:
  - For every use of a vtable, Sema calls MarkVTableUsed() to indicate
  the use. For example, this occurs when calling a virtual member
  function of the class, defining a constructor of that class type,
  dynamic_cast'ing from that type to a derived class, casting
  to/through a virtual base class, etc.
  - For every definition of a vtable, Sema calls MarkVTableUsed() to
  indicate the definition. This happens at the end of the translation
  unit for classes whose key function has been defined (so we can
  delay computation of the key function; see PR6564), and will also
  occur with explicit template instantiation definitions.
 - For every vtable defined/used, we mark all of the virtual member
 functions of that vtable as defined/used, unless we know that the key
 function is in another translation unit. This instantiates virtual
 member functions when needed.
  - At the end of the translation unit, Sema tells CodeGen (via the
  ASTConsumer) which vtables must be defined (CodeGen will define
  them) and which may be used (for which CodeGen will define the
  vtables lazily). 

From a language perspective, both the old and the new schemes are
permissible: we're allowed to instantiate virtual member functions
whenever we want per the standard. However, all other C++ compilers
were more lazy than we were, and our eagerness was both a performance
issue (we instantiated too much) and a portability problem (we broke
Boost test cases, which now pass).

Notes:
  (1) There's a ton of churn in the tests, because the order in which
  vtables get emitted to IR has changed. I've tried to isolate some of
  the larger tests from these issues.
  (2) Some diagnostics related to
  implicitly-instantiated/implicitly-defined virtual member functions
  have moved to the point of first use/definition. It's better this
  way.
  (3) I could use a review of the places where we MarkVTableUsed, to
  see if I missed any place where the language effectively requires a
  vtable.

Fixes PR7114 and PR6564.

llvm-svn: 103718

clang/include/clang/AST/ASTConsumer.h

@@ -16,6 +16,7 @@
 
 namespace clang {
   class ASTContext;
+  class CXXRecordDecl;
   class DeclGroupRef;
   class TagDecl;
   class HandleTagDeclDefinition;
@@ -68,6 +69,17 @@ class ASTConsumer {
   /// modified by the introduction of an implicit zero initializer.
   virtual void CompleteTentativeDefinition(VarDecl *D) {}
 
+  /// \brief Callback involved at the end of a translation unit to
+  /// notify the consumer that a vtable for the given C++ class is
+  /// required.
+  ///
+  /// \param RD The class whose vtable was used.
+  ///
+  /// \param DefinitionRequired Whether a definition of this vtable is
+  /// required in this translation unit; otherwise, it is only needed if
+  /// it was actually used.
+  virtual void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) {}
+
   /// PrintStats - If desired, print any statistics.
   virtual void PrintStats() {}
 

clang/lib/CodeGen/CGVTables.cpp

@@ -2788,7 +2788,13 @@ void CodeGenVTables::ComputeVTableRelatedInformation(const CXXRecordDecl *RD) {
   // Check if we've computed this information before.
   if (LayoutData)
     return;
-
+
+  // We may need to generate a definition for this vtable.
+  if (!isKeyFunctionInAnotherTU(CGM.getContext(), RD) &&
+      RD->getTemplateSpecializationKind()
+                                      != TSK_ExplicitInstantiationDeclaration)
+    CGM.DeferredVTables.push_back(RD);
+
   VTableBuilder Builder(*this, RD, 0, /*MostDerivedClassIsVirtual=*/0, RD);
 
   // Add the VTable layout.
@@ -3119,49 +3125,3 @@ CodeGenVTables::GenerateClassData(llvm::GlobalVariable::LinkageTypes Linkage,
       DC->getParent()->isTranslationUnit())
     CGM.EmitFundamentalRTTIDescriptors();
 }
-
-void CodeGenVTables::EmitVTableRelatedData(GlobalDecl GD) {
-  const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
-  const CXXRecordDecl *RD = MD->getParent();
-
-  // If the class doesn't have a vtable we don't need to emit one.
-  if (!RD->isDynamicClass())
-    return;
-
-  // Check if we need to emit thunks for this function.
-  if (MD->isVirtual())
-    EmitThunks(GD);
-
-  // Get the key function.
-  const CXXMethodDecl *KeyFunction = CGM.getContext().getKeyFunction(RD);
-
-  TemplateSpecializationKind RDKind = RD->getTemplateSpecializationKind();
-  TemplateSpecializationKind MDKind = MD->getTemplateSpecializationKind();
-
-  if (KeyFunction) {
-    // We don't have the right key function.
-    if (KeyFunction->getCanonicalDecl() != MD->getCanonicalDecl())
-      return;
-  } else {
-    // If we have no key function and this is a explicit instantiation 
-    // declaration, we will produce a vtable at the explicit instantiation. We 
-    // don't need one here.
-    if (RDKind == clang::TSK_ExplicitInstantiationDeclaration)
-      return;
-
-    // If this is an explicit instantiation of a method, we don't need a vtable.
-    // Since we have no key function, we will emit the vtable when we see
-    // a use, and just defining a function is not an use.
-    if (RDKind == TSK_ImplicitInstantiation &&
-        MDKind == TSK_ExplicitInstantiationDefinition)
-      return;
-  }
-
-  if (VTables.count(RD))
-    return;
-
-  if (RDKind == TSK_ImplicitInstantiation)
-    CGM.DeferredVTables.push_back(RD);
-  else
-    GenerateClassData(CGM.getVTableLinkage(RD), RD);
-}

clang/lib/CodeGen/CGVTables.h

@@ -272,9 +272,6 @@ class CodeGenVTables {
   /// EmitThunk - Emit a single thunk.
   void EmitThunk(GlobalDecl GD, const ThunkInfo &Thunk);
 
-  /// EmitThunks - Emit the associated thunks for the given global decl.
-  void EmitThunks(GlobalDecl GD);
-
   /// ComputeVTableRelatedInformation - Compute and store all vtable related
   /// information (vtable layout, vbase offset offsets, thunks etc) for the
   /// given record decl.
@@ -349,11 +346,10 @@ class CodeGenVTables {
                              VTableAddressPointsMapTy& AddressPoints);
 
   llvm::GlobalVariable *getVTT(const CXXRecordDecl *RD);
-
-  // EmitVTableRelatedData - Will emit any thunks that the global decl might
-  // have, as well as the vtable itself if the global decl is the key function.
-  void EmitVTableRelatedData(GlobalDecl GD);
 
+  /// EmitThunks - Emit the associated thunks for the given global decl.
+  void EmitThunks(GlobalDecl GD);
+
   /// GenerateClassData - Generate all the class data required to be generated
   /// upon definition of a KeyFunction.  This includes the vtable, the
   /// rtti data structure and the VTT.

clang/lib/CodeGen/CodeGenModule.cpp

@@ -727,8 +727,9 @@ void CodeGenModule::EmitGlobalDefinition(GlobalDecl GD) {
                                  Context.getSourceManager(),
                                  "Generating code for declaration");
 
-  if (isa<CXXMethodDecl>(D))
-    getVTables().EmitVTableRelatedData(GD);
+  if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D))
+    if (Method->isVirtual())
+      getVTables().EmitThunks(GD);
 
   if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(D))
     return EmitCXXConstructor(CD, GD.getCtorType());
@@ -984,6 +985,11 @@ void CodeGenModule::EmitTentativeDefinition(const VarDecl *D) {
   EmitGlobalVarDefinition(D);
 }
 
+void CodeGenModule::EmitVTable(CXXRecordDecl *Class, bool DefinitionRequired) {
+  if (DefinitionRequired)
+    getVTables().GenerateClassData(getVTableLinkage(Class), Class);
+}
+
 llvm::GlobalVariable::LinkageTypes 
 CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
   if (RD->isInAnonymousNamespace() || !RD->hasLinkage())

clang/lib/CodeGen/CodeGenModule.h

@@ -422,6 +422,8 @@ class CodeGenModule : public BlockModule {
 
   void EmitTentativeDefinition(const VarDecl *D);
 
+  void EmitVTable(CXXRecordDecl *Class, bool DefinitionRequired);
+
   enum GVALinkage {
     GVA_Internal,
     GVA_C99Inline,

clang/lib/CodeGen/ModuleBuilder.cpp

@@ -89,6 +89,13 @@ namespace {
 
       Builder->EmitTentativeDefinition(D);
     }
+
+    virtual void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) {
+      if (Diags.hasErrorOccurred())
+        return;
+
+      Builder->EmitVTable(RD, DefinitionRequired);
+    }
   };
 }
 

clang/lib/Frontend/CodeGenAction.cpp

@@ -180,6 +180,10 @@ namespace {
       Gen->CompleteTentativeDefinition(D);
     }
 
+    virtual void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) {
+      Gen->HandleVTable(RD, DefinitionRequired);
+    }
+
     static void InlineAsmDiagHandler(const llvm::SMDiagnostic &SM,void *Context,
                                      unsigned LocCookie) {
       SourceLocation Loc = SourceLocation::getFromRawEncoding(LocCookie);

clang/lib/Sema/Sema.cpp

@@ -164,6 +164,18 @@ void Sema::ImpCastExprToType(Expr *&Expr, QualType Ty,
     }
   }
 
+  // If this is a derived-to-base cast to a through a virtual base, we
+  // need a vtable.
+  if (Kind == CastExpr::CK_DerivedToBase && 
+      BasePathInvolvesVirtualBase(BasePath)) {
+    QualType T = Expr->getType();
+    if (const PointerType *Pointer = T->getAs<PointerType>())
+      T = Pointer->getPointeeType();
+    if (const RecordType *RecordTy = T->getAs<RecordType>())
+      MarkVTableUsed(Expr->getLocStart(), 
+                     cast<CXXRecordDecl>(RecordTy->getDecl()));
+  }
+
   if (ImplicitCastExpr *ImpCast = dyn_cast<ImplicitCastExpr>(Expr)) {
     if (ImpCast->getCastKind() == Kind && BasePath.empty()) {
       ImpCast->setType(Ty);
@@ -199,10 +211,10 @@ void Sema::ActOnEndOfTranslationUnit() {
     // template instantiations earlier.
     PerformPendingImplicitInstantiations();
 
-    /// If ProcessPendingClassesWithUnmarkedVirtualMembers ends up marking 
-    /// any virtual member functions it might lead to more pending template
+    /// If DefinedUsedVTables ends up marking any virtual member
+    /// functions it might lead to more pending template
     /// instantiations, which is why we need to loop here.
-    if (!ProcessPendingClassesWithUnmarkedVirtualMembers())
+    if (!DefineUsedVTables())
       break;
   }
 

clang/lib/Sema/Sema.h

@@ -2560,27 +2560,38 @@ class Sema : public Action {
   void MarkBaseAndMemberDestructorsReferenced(SourceLocation Loc,
                                               CXXRecordDecl *Record);
 
-  /// ClassesWithUnmarkedVirtualMembers - Contains record decls whose virtual
-  /// members need to be marked as referenced at the end of the translation
-  /// unit. It will contain polymorphic classes that do not have a key
-  /// function or have a key function that has been defined.
-  llvm::SmallVector<std::pair<CXXRecordDecl *, SourceLocation>, 4>
-    ClassesWithUnmarkedVirtualMembers;
-
-  /// MaybeMarkVirtualMembersReferenced - If the passed in method is the
-  /// key function of the record decl, will mark virtual member functions as
-  /// referenced.
-  void MaybeMarkVirtualMembersReferenced(SourceLocation Loc, CXXMethodDecl *MD);
+  /// \brief The list of classes whose vtables have been used within
+  /// this translation unit, and the source locations at which the
+  /// first use occurred.
+  llvm::SmallVector<std::pair<CXXRecordDecl *, SourceLocation>, 16> 
+    VTableUses;
+
+  /// \brief The set of classes whose vtables have been used within
+  /// this translation unit, and a bit that will be true if the vtable is
+  /// required to be emitted (otherwise, it should be emitted only if needed
+  /// by code generation).
+  llvm::DenseMap<CXXRecordDecl *, bool> VTablesUsed;
+
+  /// \brief A list of all of the dynamic classes in this translation
+  /// unit.
+  llvm::SmallVector<CXXRecordDecl *, 16> DynamicClasses;
+
+  /// \brief Note that the vtable for the given class was used at the
+  /// given location.
+  void MarkVTableUsed(SourceLocation Loc, CXXRecordDecl *Class,
+                      bool DefinitionRequired = false);
 
   /// MarkVirtualMembersReferenced - Will mark all virtual members of the given
   /// CXXRecordDecl referenced.
   void MarkVirtualMembersReferenced(SourceLocation Loc,
                                     const CXXRecordDecl *RD);
 
-  /// ProcessPendingClassesWithUnmarkedVirtualMembers - Will process classes
-  /// that might need to have their virtual members marked as referenced.
-  /// Returns false if no work was done.
-  bool ProcessPendingClassesWithUnmarkedVirtualMembers();
+  /// \brief Define all of the vtables that have been used in this
+  /// translation unit and reference any virtual members used by those
+  /// vtables.
+  ///
+  /// \returns true if any work was done, false otherwise.
+  bool DefineUsedVTables();
 
   void AddImplicitlyDeclaredMembersToClass(Scope *S, CXXRecordDecl *ClassDecl);
 
@@ -2664,6 +2675,8 @@ class Sema : public Action {
   void BuildBasePathArray(const CXXBasePaths &Paths,
                           CXXBaseSpecifierArray &BasePath);
 
+  bool BasePathInvolvesVirtualBase(const CXXBaseSpecifierArray &BasePath);
+
   bool CheckDerivedToBaseConversion(QualType Derived, QualType Base,
                                     SourceLocation Loc, SourceRange Range,
                                     CXXBaseSpecifierArray *BasePath = 0,

clang/lib/Sema/SemaCXXCast.cpp

@@ -388,6 +388,12 @@ CheckDynamicCast(Sema &Self, Expr *&SrcExpr, QualType DestType,
         return;
 
     Kind = CastExpr::CK_DerivedToBase;
+
+    // If we are casting to or through a virtual base class, we need a
+    // vtable.
+    if (Self.BasePathInvolvesVirtualBase(BasePath))
+      Self.MarkVTableUsed(OpRange.getBegin(), 
+                          cast<CXXRecordDecl>(SrcRecord->getDecl()));
     return;
   }
 
@@ -398,6 +404,8 @@ CheckDynamicCast(Sema &Self, Expr *&SrcExpr, QualType DestType,
     Self.Diag(OpRange.getBegin(), diag::err_bad_dynamic_cast_not_polymorphic)
       << SrcPointee.getUnqualifiedType() << SrcExpr->getSourceRange();
   }
+  Self.MarkVTableUsed(OpRange.getBegin(), 
+                      cast<CXXRecordDecl>(SrcRecord->getDecl()));
 
   // Done. Everything else is run-time checks.
   Kind = CastExpr::CK_Dynamic;

clang/lib/Sema/SemaDecl.cpp

@@ -4575,12 +4575,14 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg,
       WP.disableCheckFallThrough();
     }
 
-    if (!FD->isInvalidDecl())
+    if (!FD->isInvalidDecl()) {
       DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
-
-    if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FD))
-      MaybeMarkVirtualMembersReferenced(Method->getLocation(), Method);
-
+
+      // If this is a constructor, we need a vtable.
+      if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(FD))
+        MarkVTableUsed(FD->getLocation(), Constructor->getParent());
+    }
+
     assert(FD == getCurFunctionDecl() && "Function parsing confused");
   } else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(dcl)) {
     assert(MD == getCurMethodDecl() && "Method parsing confused");
@@ -5447,37 +5449,6 @@ void Sema::ActOnStartCXXMemberDeclarations(Scope *S, DeclPtrTy TagD,
          "Broken injected-class-name");
 }
 
-// Traverses the class and any nested classes, making a note of any 
-// dynamic classes that have no key function so that we can mark all of
-// their virtual member functions as "used" at the end of the translation
-// unit. This ensures that all functions needed by the vtable will get
-// instantiated/synthesized.
-static void 
-RecordDynamicClassesWithNoKeyFunction(Sema &S, CXXRecordDecl *Record,
-                                      SourceLocation Loc) {
-  // We don't look at dependent or undefined classes.
-  if (Record->isDependentContext() || !Record->isDefinition())
-    return;
-
-  if (Record->isDynamicClass()) {
-    const CXXMethodDecl *KeyFunction = S.Context.getKeyFunction(Record);
-
-    if (!KeyFunction)
-      S.ClassesWithUnmarkedVirtualMembers.push_back(std::make_pair(Record,
-                                                                   Loc));
-
-    if ((!KeyFunction || (KeyFunction->getBody() && KeyFunction->isInlined()))
-        && Record->getLinkage() == ExternalLinkage)
-      S.Diag(Record->getLocation(), diag::warn_weak_vtable) << Record;
-  }
-  for (DeclContext::decl_iterator D = Record->decls_begin(), 
-                               DEnd = Record->decls_end();
-       D != DEnd; ++D) {
-    if (CXXRecordDecl *Nested = dyn_cast<CXXRecordDecl>(*D))
-      RecordDynamicClassesWithNoKeyFunction(S, Nested, Loc);
-  }
-}
-
 void Sema::ActOnTagFinishDefinition(Scope *S, DeclPtrTy TagD,
                                     SourceLocation RBraceLoc) {
   AdjustDeclIfTemplate(TagD);
@@ -5489,10 +5460,6 @@ void Sema::ActOnTagFinishDefinition(Scope *S, DeclPtrTy TagD,
 
   // Exit this scope of this tag's definition.
   PopDeclContext();
-
-  if (isa<CXXRecordDecl>(Tag) && !Tag->getLexicalDeclContext()->isRecord())
-    RecordDynamicClassesWithNoKeyFunction(*this, cast<CXXRecordDecl>(Tag),
-                                          RBraceLoc);
 
   // Notify the consumer that we've defined a tag.
   Consumer.HandleTagDeclDefinition(Tag);