[cfe-dev] Researching tying non-virtual inline function definitions to vtables

[David Blaikie via cfe-dev]

Hi Richard (& others),

As a tangent from:
http://clang-developers.42468.n3.nabble.com/Could-we-emit-inline-virtual-member-functions-as-available-externally-when-there-s-a-key-function-sa-td4051908.html

I'm trying to see whether we can go further than that: Specifically,
emitting even non-virtual inline functions for dynamic classes alongside
the class's vtable (& omitting them elsewhere (except when optimizations
are enabled, and then emitting them available_externally))

I tried the attached patch as a total scrappy hack to get numbers. It only
implements one side of this (well, first it implements the ABI-breaking but
language-conforming thing mentioned in the original thread, of doing inline
virtual functions the same as the vtable) by omitting the definition of
inline member functions of a type with a vtable where the vtable isn't
emitted. It does emit strong definitions of any inline function that is
used in the TU where the vtable is emitted - but fails to emit the strong
definitions otherwise.

So at best, if I look at object size when using this patch, I should see a
greater benefit than is actually possible (the benefit will shrink once the
strong definitions are properly emitted).

Does this patch look even approximately right for that task? For my small
test cases it seems to be, and judging by the linker errors it looks like
it's doing approximately the right thing in larger test cases.

But I'm only seeing about a 2% reduction in object size over a large
binary, which seemed surprisingly low. Plausible? Is this obviously missing
some opportunities? Any ideas on ways I could investigate this further?

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160615/5aa1dbd5/attachment.html>
-------------- next part --------------
diff --git lib/CodeGen/CGVTables.cpp lib/CodeGen/CGVTables.cpp
index 2a742a9..7bac99e 100644
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -721,10 +721,37 @@ static bool shouldEmitAvailableExternallyVTable(const CodeGenModule &CGM,
          CGM.getCXXABI().canSpeculativelyEmitVTable(RD);
 }
 
+/// Given that we're currently at the end of the translation unit, and
+/// we've emitted a reference to the vtable for this class, should
+/// we define that vtable?
+static bool shouldEmitVTableAtEndOfTranslationUnit(CodeGenModule &CGM,
+                                                   const CXXRecordDecl *RD) {
+  // If vtable is internal then it has to be done.
+  if (!CGM.getVTables().isVTableExternal(RD))
+    return true;
+
+  // If it's external then maybe we will need it as available_externally.
+  return shouldEmitAvailableExternallyVTable(CGM, RD);
+}
+
+llvm::GlobalValue::LinkageTypes
+CodeGenModule::getInlineMemberFunctionLinkage(const CXXMethodDecl *M, llvm::GlobalValue::LinkageTypes Original) {
+  if (Original != llvm::GlobalValue::LinkOnceODRLinkage || M->isImplicit())
+    return Original;
+  if (!shouldEmitVTableAtEndOfTranslationUnit(*this, M->getParent()))
+    return llvm::GlobalValue::AvailableExternallyLinkage;
+  auto VLink = getVTableLinkage(M->getParent());
+  if (VLink == llvm::GlobalValue::ExternalLinkage)
+    return VLink;
+  if (VLink == llvm::GlobalValue::AvailableExternallyLinkage)
+    return VLink;
+  return Original;
+}
+
 /// Compute the required linkage of the vtable for the given class.
 ///
 /// Note that we only call this at the end of the translation unit.
-llvm::GlobalVariable::LinkageTypes 
+llvm::GlobalValue::LinkageTypes 
 CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
   if (!RD->isExternallyVisible())
     return llvm::GlobalVariable::InternalLinkage;
@@ -868,19 +895,6 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
   return !keyFunction->hasBody();
 }
 
-/// Given that we're currently at the end of the translation unit, and
-/// we've emitted a reference to the vtable for this class, should
-/// we define that vtable?
-static bool shouldEmitVTableAtEndOfTranslationUnit(CodeGenModule &CGM,
-                                                   const CXXRecordDecl *RD) {
-  // If vtable is internal then it has to be done.
-  if (!CGM.getVTables().isVTableExternal(RD))
-    return true;
-
-  // If it's external then maybe we will need it as available_externally.
-  return shouldEmitAvailableExternallyVTable(CGM, RD);
-}
-
 /// Given that at some point we emitted a reference to one or more
 /// vtables, and that we are now at the end of the translation unit,
 /// decide whether we should emit them.
diff --git lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.cpp
index badc593..a085cd7 100644
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -769,7 +769,10 @@ CodeGenModule::getFunctionLinkage(GlobalDecl GD) {
                                    : llvm::GlobalValue::LinkOnceODRLinkage;
   }
 
-  return getLLVMLinkageForDeclarator(D, Linkage, /*isConstantVariable=*/false);
+  auto Result = getLLVMLinkageForDeclarator(D, Linkage, /*isConstantVariable=*/false);
+  if (const auto *M = dyn_cast<CXXMethodDecl>(D))
+    return getInlineMemberFunctionLinkage(M, Result);
+  return Result;
 }
 
 void CodeGenModule::setFunctionDLLStorageClass(GlobalDecl GD, llvm::Function *F) {
diff --git lib/CodeGen/CodeGenModule.h lib/CodeGen/CodeGenModule.h
index a75e4aa..69f855f 100644
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1018,9 +1018,11 @@ public:
   /// Set the DLL storage class on F.
   void setFunctionDLLStorageClass(GlobalDecl GD, llvm::Function *F);
 
+  llvm::GlobalValue::LinkageTypes getInlineMemberFunctionLinkage(const CXXMethodDecl *M, llvm::GlobalValue::LinkageTypes Original);
+
   /// Return the appropriate linkage for the vtable, VTT, and type information
   /// of the given class.
-  llvm::GlobalVariable::LinkageTypes getVTableLinkage(const CXXRecordDecl *RD);
+  llvm::GlobalValue::LinkageTypes getVTableLinkage(const CXXRecordDecl *RD);
 
   /// Return the store size, in character units, of the given LLVM type.
   CharUnits GetTargetTypeStoreSize(llvm::Type *Ty) const;
diff --git lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/ItaniumCXXABI.cpp
index 4da7b94..13db447 100644
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1638,8 +1638,9 @@ bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
   // then we are safe to emit available_externally copy of vtable.
   // FIXME we can still emit a copy of the vtable if we
   // can emit definition of the inline functions.
-  return !hasAnyUsedVirtualInlineFunction(RD) && !isVTableHidden(RD);
+  return !isVTableHidden(RD);
 }
+
 static llvm::Value *performTypeAdjustment(CodeGenFunction &CGF,
                                           Address InitialPtr,
                                           int64_t NonVirtualAdjustment,