v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, bruno.
Herald added a subscriber: cfe-commits.
If the ABI supports inline key functions we must emit them eagerly. The generic
reason is that CodeGen might assume they are emitted and generate a reference
to the vtable. In deserialization this rule becomes a little less clear because
if a reference is generated, CodeGen will make a request and we will emit the
vtable on demand.
This patch improves the performance of loading modules by slimming down the
EAGERLY_DESERIALIZED_DECLS sections. It is equivalent to 'pinning' the vtable,
with two benefits: (a) it does not require expertise in modules to find out
that outlining the key function brings performance; (b) it also helps with
third-party codebases.
We ran this change through our codebase and we saw no observable changes in
behavior.
Patch by Yuka Takahashi and me!
Repository:
rC Clang
https://reviews.llvm.org/D53925
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/Serialization/ASTWriterDecl.cpp
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -2264,6 +2264,19 @@
return false;
}
+ // If the ABI supports inline key functions we must emit them eagerly. The
+ // generic reason is that CodeGen might assume they are emitted and generate
a
+ // reference to the vtable. In deserialization this rule becomes a little
less
+ // clear because if a reference is generated, CodeGen will make a request and
+ // we will emit the vtable on demand.
+ //
+ // This change in behavior is driven mostly by performance considerations and
+ // is equivalent in outlining the key function (aka pinning the vtable). It
+ // also works in cases where we cannot modify the source code.
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))
+ if (Context.isKeyFunction(MD))
+ return false;
+
return Context.DeclMustBeEmitted(D);
}
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9700,6 +9700,20 @@
basicGVALinkageForVariable(*this, VD)));
}
+bool ASTContext::isKeyFunction(const CXXMethodDecl *MD) {
+ assert(MD);
+
+ if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
+ const CXXRecordDecl *RD = MD->getParent();
+ if (MD->isOutOfLine() && RD->isDynamicClass()) {
+ const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
+ if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
+ return true;
+ }
+ }
+ return false;
+}
+
bool ASTContext::DeclMustBeEmitted(const Decl *D) {
if (const auto *VD = dyn_cast<VarDecl>(D)) {
if (!VD->isFileVarDecl())
@@ -9781,18 +9795,11 @@
if (FD->hasAttr<ConstructorAttr>() || FD->hasAttr<DestructorAttr>())
return true;
- // The key function for a class is required. This rule only comes
+ // The key function for a class is required. This rule only comes
// into play when inline functions can be key functions, though.
- if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
- if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
- const CXXRecordDecl *RD = MD->getParent();
- if (MD->isOutOfLine() && RD->isDynamicClass()) {
- const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
- if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
- return true;
- }
- }
- }
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD))
+ if (isKeyFunction(MD))
+ return true;
GVALinkage Linkage = GetGVALinkageForFunction(FD);
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2733,6 +2733,12 @@
GVALinkage GetGVALinkageForFunction(const FunctionDecl *FD) const;
GVALinkage GetGVALinkageForVariable(const VarDecl *VD);
+ /// Determines if the decl is the key function for a class.
+ ///
+ /// \ returns true if the function is the key and it was inline requiring
+ /// emission even if it is not used.
+ bool isKeyFunction(const CXXMethodDecl *MD);
+
/// Determines if the decl can be CodeGen'ed or deserialized from PCH
/// lazily, only when used; this is only relevant for function or file scoped
/// var definitions.
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -2264,6 +2264,19 @@
return false;
}
+ // If the ABI supports inline key functions we must emit them eagerly. The
+ // generic reason is that CodeGen might assume they are emitted and generate a
+ // reference to the vtable. In deserialization this rule becomes a little less
+ // clear because if a reference is generated, CodeGen will make a request and
+ // we will emit the vtable on demand.
+ //
+ // This change in behavior is driven mostly by performance considerations and
+ // is equivalent in outlining the key function (aka pinning the vtable). It
+ // also works in cases where we cannot modify the source code.
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))
+ if (Context.isKeyFunction(MD))
+ return false;
+
return Context.DeclMustBeEmitted(D);
}
Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -9700,6 +9700,20 @@
basicGVALinkageForVariable(*this, VD)));
}
+bool ASTContext::isKeyFunction(const CXXMethodDecl *MD) {
+ assert(MD);
+
+ if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
+ const CXXRecordDecl *RD = MD->getParent();
+ if (MD->isOutOfLine() && RD->isDynamicClass()) {
+ const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
+ if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
+ return true;
+ }
+ }
+ return false;
+}
+
bool ASTContext::DeclMustBeEmitted(const Decl *D) {
if (const auto *VD = dyn_cast<VarDecl>(D)) {
if (!VD->isFileVarDecl())
@@ -9781,18 +9795,11 @@
if (FD->hasAttr<ConstructorAttr>() || FD->hasAttr<DestructorAttr>())
return true;
- // The key function for a class is required. This rule only comes
+ // The key function for a class is required. This rule only comes
// into play when inline functions can be key functions, though.
- if (getTargetInfo().getCXXABI().canKeyFunctionBeInline()) {
- if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
- const CXXRecordDecl *RD = MD->getParent();
- if (MD->isOutOfLine() && RD->isDynamicClass()) {
- const CXXMethodDecl *KeyFunc = getCurrentKeyFunction(RD);
- if (KeyFunc && KeyFunc->getCanonicalDecl() == MD->getCanonicalDecl())
- return true;
- }
- }
- }
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD))
+ if (isKeyFunction(MD))
+ return true;
GVALinkage Linkage = GetGVALinkageForFunction(FD);
Index: include/clang/AST/ASTContext.h
===================================================================
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2733,6 +2733,12 @@
GVALinkage GetGVALinkageForFunction(const FunctionDecl *FD) const;
GVALinkage GetGVALinkageForVariable(const VarDecl *VD);
+ /// Determines if the decl is the key function for a class.
+ ///
+ /// \ returns true if the function is the key and it was inline requiring
+ /// emission even if it is not used.
+ bool isKeyFunction(const CXXMethodDecl *MD);
+
/// Determines if the decl can be CodeGen'ed or deserialized from PCH
/// lazily, only when used; this is only relevant for function or file scoped
/// var definitions.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
