[PATCH] D33437: Emit available_externally vtables opportunistically
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304394: Emit available_externally vtables opportunistically
(authored by Prazek).
Changed prior to commit:
https://reviews.llvm.org/D33437?vs=100641&id=100976#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33437
Files:
cfe/trunk/include/clang/AST/VTableBuilder.h
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp
cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
Index: cfe/trunk/lib/CodeGen/CodeGenModule.h
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.h
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h
@@ -341,6 +341,9 @@
/// A queue of (optional) vtables to consider emitting.
std::vector DeferredVTables;
+ /// A queue of (optional) vtables that may be emitted opportunistically.
+ std::vector OpportunisticVTables;
+
/// List of global values which are required to be present in the object file;
/// bitcast to i8*. This is used for forcing visibility of symbols which may
/// otherwise be optimized out.
@@ -450,7 +453,7 @@
bool isTriviallyRecursive(const FunctionDecl *F);
bool shouldEmitFunction(GlobalDecl GD);
-
+ bool shouldOpportunisticallyEmitVTables();
/// Map used to be sure we don't emit the same CompoundLiteral twice.
llvm::DenseMap
EmittedCompoundLiterals;
@@ -1278,6 +1281,12 @@
/// Emit any needed decls for which code generation was deferred.
void EmitDeferred();
+ /// Try to emit external vtables as available_externally if they have emitted
+ /// all inlined virtual functions. It runs after EmitDeferred() and therefore
+ /// is not allowed to create new references to things that need to be emitted
+ /// lazily.
+ void EmitVTablesOpportunistically();
+
/// Call replaceAllUsesWith on all pairs in Replacements.
void applyReplacements();
Index: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
===
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
private:
- bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto &VtableLayout =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto &VtableComponent : VtableLayout.vtable_components()) {
- // Skip empty slot.
- if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
- const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
- if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+ bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto &VtableLayout =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto &VtableComponent : VtableLayout.vtable_components()) {
+ // Skip empty slot.
+ if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+ if (!Method->getCanonicalDecl()->isInlined())
+ continue;
+
+ StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl());
+ auto *Entry = CGM.GetGlobalValue(Name);
+ // This checks if virtual inline function has already been emitted.
+ // Note that it is possible that this inline function would be emitted
+ // after trying to emit vtable speculatively. Because of this we do
+ // an extra pass after emitting all deferred vtables to find and emit
+ // these vtables opportunistically.
+ if (!Entry || Entry->isDeclaration())
+ return true;
+ }
+ return false;
}
bool isVTableHidden(const CXXRecordDecl *RD) const {
@@ -1687,11 +1697,11 @@
if (CGM.getLangOpts().AppleKext)
return false;
- // If we don't have any inline virtual functions, and if vtable is not hidden,
- // then we are safe to emit available_externally copy of vtable.
+ // If we don't have any not emitted inline virtual function, and if vtable is
+ // not hidden, 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 !hasAnyVirtualInlineFunction(RD) && !isVTableHidden(RD);
+ return !hasAnyUnusedVirtualInlineFunction(RD) && !isVTableHidden(RD);
}
static llvm::Value *performTypeAdjustment(CodeGenFunction &CGF,
Address InitialPtr,
@@ -2576,6 +2586,9 @@
if (!GV) {
// Create a new global variable.
+// Note for the future: If we would ever like to do deferred em
[PATCH] D33437: Emit available_externally vtables opportunistically
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Looks great, thanks! https://reviews.llvm.org/D33437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek updated this revision to Diff 100641.
Prazek added a comment.
changed assert
https://reviews.llvm.org/D33437
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/vtable-available-externally.cpp
test/CodeGenCXX/vtable-linkage.cpp
Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -145,12 +145,14 @@
// F is an explicit template instantiation declaration without a
// key function, so its vtable should have external linkage.
// CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant
+// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant
// E is an explicit template instantiation declaration. It has a
// key function is not instantiated, so we know that vtable definition
// will be generated in TU where key function will be defined
-// so we can mark it as available_externally (only with optimizations)
+// so we can mark it as external (without optimizations) and
+// available_externally (with optimizations) because all of the inline
+// virtual functions have been emitted.
// CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant
// CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant
Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -12,6 +12,7 @@
// RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
#include
@@ -274,8 +275,8 @@
virtual D& operator=(const D&);
};
-// Cannot emit B's vtable available_externally, because we cannot create
-// a reference to the inline virtual B::operator= function.
+// Cannot emit D's vtable available_externally, because we cannot create
+// a reference to the inline virtual D::operator= function.
// CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant
struct D : C {
virtual void key();
@@ -391,3 +392,30 @@
}
}
+namespace Test17 {
+// This test checks if we emit vtables opportunistically.
+// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+
+struct A {
+ virtual void key();
+ virtual void bar() {}
+};
+
+// We won't gonna use deleting destructor for this type, which will disallow
+// emitting vtable as available_externally
+struct B {
+ virtual void key();
+ virtual ~B() {}
+};
+
+void testcaseA() {
+ A a;
+ a.bar(); // this forces to emit definition of bar
+}
+
+void testcaseB() {
+ B b; // This only forces emitting of complete object destructor
+}
+
+} // namespace Test17
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3756,6 +3756,9 @@
if (llvm::GlobalVariable *GV = CGM.getModule().getNamedGlobal(MangledName))
return llvm::ConstantExpr::getBitCast(GV, CGM.Int8PtrTy);
+ // Note for the future: If we would ever like to do deferred emission of
+ // RTTI, check if emitting vtables opportunistically need any adjustment.
+
// Compute the fields for the TypeDescriptor.
SmallString<256> TypeInfoString;
{
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
private:
- bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto &VtableLayout =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto &VtableComponent : VtableLayout.vtable_components()) {
- // Skip empty slot.
- if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
- const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
- if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+ bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto &VtableLayout =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto &VtableComponent : VtableLayout.vtable_components()) {
+ // Skip empty slot.
+ if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+ if (!Method->getCanonicalDecl()->is
[PATCH] D33437: Emit available_externally vtables opportunistically
dblaikie added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1383-1385 + if (!OpportunisticVTables.empty()) +assert(shouldOpportunisticallyEmitVTables() && + "Only emit opportunistic vtables with optimizations"); Perhaps this: assert(OpportunisticVTables.empty() || shouldOpportunisticallyEmitVTables() && ... ) (it's a bit odd to have a condition that only goes to an assert - rather than having both conditions inside the assertion) https://reviews.llvm.org/D33437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek updated this revision to Diff 100623.
Prazek marked an inline comment as done.
Prazek added a comment.
- Final changes
https://reviews.llvm.org/D33437
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/vtable-available-externally.cpp
test/CodeGenCXX/vtable-linkage.cpp
Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -145,12 +145,14 @@
// F is an explicit template instantiation declaration without a
// key function, so its vtable should have external linkage.
// CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant
+// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant
// E is an explicit template instantiation declaration. It has a
// key function is not instantiated, so we know that vtable definition
// will be generated in TU where key function will be defined
-// so we can mark it as available_externally (only with optimizations)
+// so we can mark it as external (without optimizations) and
+// available_externally (with optimizations) because all of the inline
+// virtual functions have been emitted.
// CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant
// CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant
Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -12,6 +12,7 @@
// RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
#include
@@ -274,8 +275,8 @@
virtual D& operator=(const D&);
};
-// Cannot emit B's vtable available_externally, because we cannot create
-// a reference to the inline virtual B::operator= function.
+// Cannot emit D's vtable available_externally, because we cannot create
+// a reference to the inline virtual D::operator= function.
// CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant
struct D : C {
virtual void key();
@@ -391,3 +392,30 @@
}
}
+namespace Test17 {
+// This test checks if we emit vtables opportunistically.
+// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+
+struct A {
+ virtual void key();
+ virtual void bar() {}
+};
+
+// We won't gonna use deleting destructor for this type, which will disallow
+// emitting vtable as available_externally
+struct B {
+ virtual void key();
+ virtual ~B() {}
+};
+
+void testcaseA() {
+ A a;
+ a.bar(); // this forces to emit definition of bar
+}
+
+void testcaseB() {
+ B b; // This only forces emitting of complete object destructor
+}
+
+} // namespace Test17
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3756,6 +3756,9 @@
if (llvm::GlobalVariable *GV = CGM.getModule().getNamedGlobal(MangledName))
return llvm::ConstantExpr::getBitCast(GV, CGM.Int8PtrTy);
+ // Note for the future: If we would ever like to do deferred emission of
+ // RTTI, check if emitting vtables opportunistically need any adjustment.
+
// Compute the fields for the TypeDescriptor.
SmallString<256> TypeInfoString;
{
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
private:
- bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto &VtableLayout =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto &VtableComponent : VtableLayout.vtable_components()) {
- // Skip empty slot.
- if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
- const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
- if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+ bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto &VtableLayout =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto &VtableComponent : VtableLayout.vtable_components()) {
+ // Skip empty slot.
+ if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek marked 4 inline comments as done. Prazek added inline comments. Comment at: include/clang/AST/VTableBuilder.h:160 + "GlobalDecl can be created only from virtual function"); +if (getKind() == CK_FunctionPointer) + return GlobalDecl(getFunctionDecl()); rjmccall wrote: > Prazek wrote: > > rjmccall wrote: > > > Please use an exhaustive switch. You can put llvm_unreachable in the > > > other cases. > > Should I implement this for RTTI fields? Or maybe leave a fixme comment > > that it could also work for some other fields in vtable, but is not > > currently used? > I think your precondition of isUsedFunctionPointerKind() is fine, you don't > need to handle RTTI in this function. > > This does raise the interesting question, though, of whether this approach is > safe for lazily-emitted RTTI. I guess it currently works because IRGen > doesn't use the normal deferred-emission mechanism for RTTI objects, so if > the RTTI object is lazily-emitted, we'll just eagerly emit it instead of > potentially ending up with an illegal reference to external RTTI. You should > add a comment to the appropriate place in CGRTTI (i.e. where we fill in the > global definition) saying that this optimization may need adjustment if we > ever switch to using deferred emission for some reason. I couldn't find CGRTTI, so I added comment in Itanium and Microsoft ABI where we create RTTI fields, but maybe the CodeGenModule::GetAddrOfRTTIDescriptor would be a better place instead? I also added small note to the EmitVTablesOpportunistically about this. https://reviews.llvm.org/D33437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek marked an inline comment as done.
Prazek added inline comments.
Comment at: include/clang/AST/VTableBuilder.h:169
+ return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Deleting);
+default:
+ llvm_unreachable("Only function pointers kinds");
rjmccall wrote:
> By "exhaustive" I mean that you should list out all the cases instead of
> using default. It means that someone who adds a new kind of v-table entry
> will get alerted to fix this switch. There's only five other cases, it's not
> too bad.
Oh right, that make sense
https://reviews.llvm.org/D33437
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
rjmccall added inline comments.
Comment at: include/clang/AST/VTableBuilder.h:160
+ "GlobalDecl can be created only from virtual function");
+if (getKind() == CK_FunctionPointer)
+ return GlobalDecl(getFunctionDecl());
Prazek wrote:
> rjmccall wrote:
> > Please use an exhaustive switch. You can put llvm_unreachable in the other
> > cases.
> Should I implement this for RTTI fields? Or maybe leave a fixme comment that
> it could also work for some other fields in vtable, but is not currently used?
I think your precondition of isUsedFunctionPointerKind() is fine, you don't
need to handle RTTI in this function.
This does raise the interesting question, though, of whether this approach is
safe for lazily-emitted RTTI. I guess it currently works because IRGen doesn't
use the normal deferred-emission mechanism for RTTI objects, so if the RTTI
object is lazily-emitted, we'll just eagerly emit it instead of potentially
ending up with an illegal reference to external RTTI. You should add a comment
to the appropriate place in CGRTTI (i.e. where we fill in the global
definition) saying that this optimization may need adjustment if we ever switch
to using deferred emission for some reason.
Comment at: include/clang/AST/VTableBuilder.h:169
+ return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Deleting);
+default:
+ llvm_unreachable("Only function pointers kinds");
By "exhaustive" I mean that you should list out all the cases instead of using
default. It means that someone who adds a new kind of v-table entry will get
alerted to fix this switch. There's only five other cases, it's not too bad.
https://reviews.llvm.org/D33437
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek updated this revision to Diff 100530.
Prazek marked 3 inline comments as done.
Prazek added a comment.
- Addressing John's comments
https://reviews.llvm.org/D33437
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/vtable-available-externally.cpp
test/CodeGenCXX/vtable-linkage.cpp
Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -145,12 +145,14 @@
// F is an explicit template instantiation declaration without a
// key function, so its vtable should have external linkage.
// CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant
+// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant
// E is an explicit template instantiation declaration. It has a
// key function is not instantiated, so we know that vtable definition
// will be generated in TU where key function will be defined
-// so we can mark it as available_externally (only with optimizations)
+// so we can mark it as external (without optimizations) and
+// available_externally (with optimizations) because all of the inline
+// virtual functions have been emitted.
// CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant
// CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant
Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -12,6 +12,7 @@
// RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
#include
@@ -274,8 +275,8 @@
virtual D& operator=(const D&);
};
-// Cannot emit B's vtable available_externally, because we cannot create
-// a reference to the inline virtual B::operator= function.
+// Cannot emit D's vtable available_externally, because we cannot create
+// a reference to the inline virtual D::operator= function.
// CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant
struct D : C {
virtual void key();
@@ -391,3 +392,30 @@
}
}
+namespace Test17 {
+// This test checks if we emit vtables opportunistically.
+// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+
+struct A {
+ virtual void key();
+ virtual void bar() {}
+};
+
+// We won't gonna use deleting destructor for this type, which will disallow
+// emitting vtable as available_externally
+struct B {
+ virtual void key();
+ virtual ~B() {}
+};
+
+void testcaseA() {
+ A a;
+ a.bar(); // this forces to emit definition of bar
+}
+
+void testcaseB() {
+ B b; // This only forces emitting of complete object destructor
+}
+
+} // namespace Test17
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
private:
- bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto &VtableLayout =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto &VtableComponent : VtableLayout.vtable_components()) {
- // Skip empty slot.
- if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
- const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
- if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+ bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto &VtableLayout =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto &VtableComponent : VtableLayout.vtable_components()) {
+ // Skip empty slot.
+ if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+ if (!Method->getCanonicalDecl()->isInlined())
+ continue;
+
+ StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl());
+ auto *Entry = CGM.GetGlobalValue(Name);
+ // This checks if virtual inline function has already been emitted.
+ // Note that it is possible that this inline function would be emitted
+ // after trying to emit vtable speculatively. Because of this we do
+ // an extra pass after emitting all deferred vtables to find and emit
+ // these vtables opportunistically.
+ if (!Entry || Entry->isDeclaration())
+ r
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek added a comment. Thanks for the comments :) Comment at: include/clang/AST/VTableBuilder.h:160 + "GlobalDecl can be created only from virtual function"); +if (getKind() == CK_FunctionPointer) + return GlobalDecl(getFunctionDecl()); rjmccall wrote: > Please use an exhaustive switch. You can put llvm_unreachable in the other > cases. Should I implement this for RTTI fields? Or maybe leave a fixme comment that it could also work for some other fields in vtable, but is not currently used? https://reviews.llvm.org/D33437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
rjmccall added inline comments.
Comment at: include/clang/AST/VTableBuilder.h:160
+ "GlobalDecl can be created only from virtual function");
+if (getKind() == CK_FunctionPointer)
+ return GlobalDecl(getFunctionDecl());
Please use an exhaustive switch. You can put llvm_unreachable in the other
cases.
Comment at: lib/CodeGen/CGVTables.cpp:905
+else
+ OpportunisticVTables.push_back(RD);
I would add a CGM.shouldOpportunisticallyEmitVTables() that can just check the
optimization level, and then you can use that to avoid even collecting v-tables
like this if you don't need to.
Comment at: lib/CodeGen/CodeGenModule.cpp:1377
+void CodeGenModule::EmitVTablesOpportunistically() {
+ // Only emit speculated vtables with optimizations.
It's worth adding a header comment to this explaining that it runs after
EmitDeferred() and therefore is not allowed to create new references to things
that need to be emitted lazily.
https://reviews.llvm.org/D33437
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek updated this revision to Diff 99897.
Prazek added a comment.
Removed debug print
https://reviews.llvm.org/D33437
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/vtable-available-externally.cpp
test/CodeGenCXX/vtable-linkage.cpp
Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -145,12 +145,14 @@
// F is an explicit template instantiation declaration without a
// key function, so its vtable should have external linkage.
// CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant
+// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant
// E is an explicit template instantiation declaration. It has a
// key function is not instantiated, so we know that vtable definition
// will be generated in TU where key function will be defined
-// so we can mark it as available_externally (only with optimizations)
+// so we can mark it as external (without optimizations) and
+// available_externally (with optimizations) because all of the inline
+// virtual functions have been emitted.
// CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant
// CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant
Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -12,6 +12,7 @@
// RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
#include
@@ -274,8 +275,8 @@
virtual D& operator=(const D&);
};
-// Cannot emit B's vtable available_externally, because we cannot create
-// a reference to the inline virtual B::operator= function.
+// Cannot emit D's vtable available_externally, because we cannot create
+// a reference to the inline virtual D::operator= function.
// CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant
struct D : C {
virtual void key();
@@ -391,3 +392,30 @@
}
}
+namespace Test17 {
+// This test checks if we emit vtables opportunistically.
+// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+
+struct A {
+ virtual void key();
+ virtual void bar() {}
+};
+
+// We won't gonna use deleting destructor for this type, which will disallow
+// emitting vtable as available_externally
+struct B {
+ virtual void key();
+ virtual ~B() {}
+};
+
+void testcaseA() {
+ A a;
+ a.bar(); // this forces to emit definition of bar
+}
+
+void testcaseB() {
+ B b; // This only forces emitting of complete object destructor
+}
+
+} // namespace Test17
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
private:
- bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto &VtableLayout =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto &VtableComponent : VtableLayout.vtable_components()) {
- // Skip empty slot.
- if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
- const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
- if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+ bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto &VtableLayout =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto &VtableComponent : VtableLayout.vtable_components()) {
+ // Skip empty slot.
+ if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+ if (!Method->getCanonicalDecl()->isInlined())
+ continue;
+
+ StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl());
+ auto *Entry = CGM.GetGlobalValue(Name);
+ // This checks if virtual inline function has already been emitted.
+ // Note that it is possible that this inline function would be emitted
+ // after trying to emit vtable speculatively. Because of this we do
+ // an extra pass after emitting all deferred vtables to find and emit
+ // these vtables opportunistically.
+ if (!Entry || Entry->isDeclaration())
+ return true;
+ }
+ return false;
}
b
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek created this revision.
We can emit vtable definition having inline function
if they are all emitted.
https://reviews.llvm.org/D33437
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/vtable-available-externally.cpp
test/CodeGenCXX/vtable-linkage.cpp
Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -145,12 +145,14 @@
// F is an explicit template instantiation declaration without a
// key function, so its vtable should have external linkage.
// CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant
+// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant
// E is an explicit template instantiation declaration. It has a
// key function is not instantiated, so we know that vtable definition
// will be generated in TU where key function will be defined
-// so we can mark it as available_externally (only with optimizations)
+// so we can mark it as external (without optimizations) and
+// available_externally (with optimizations) because all of the inline
+// virtual functions have been emitted.
// CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant
// CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant
Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -12,6 +12,7 @@
// RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
// RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
#include
@@ -274,8 +275,8 @@
virtual D& operator=(const D&);
};
-// Cannot emit B's vtable available_externally, because we cannot create
-// a reference to the inline virtual B::operator= function.
+// Cannot emit D's vtable available_externally, because we cannot create
+// a reference to the inline virtual D::operator= function.
// CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant
struct D : C {
virtual void key();
@@ -391,3 +392,30 @@
}
}
+namespace Test17 {
+// This test checks if we emit vtables opportunistically.
+// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+
+struct A {
+ virtual void key();
+ virtual void bar() {}
+};
+
+// We won't gonna use deleting destructor for this type, which will disallow
+// emitting vtable as available_externally
+struct B {
+ virtual void key();
+ virtual ~B() {}
+};
+
+void testcaseA() {
+ A a;
+ a.bar(); // this forces to emit definition of bar
+}
+
+void testcaseB() {
+ B b; // This only forces emitting of complete object destructor
+}
+
+} // namespace Test17
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
private:
- bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto &VtableLayout =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto &VtableComponent : VtableLayout.vtable_components()) {
- // Skip empty slot.
- if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
- const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
- if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+ bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto &VtableLayout =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto &VtableComponent : VtableLayout.vtable_components()) {
+ // Skip empty slot.
+ if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+ if (!Method->getCanonicalDecl()->isInlined())
+ continue;
+
+ StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl());
+ auto *Entry = CGM.GetGlobalValue(Name);
+ // This checks if virtual inline function has already been emitted.
+ // Note that it is possible that this inline function would be emitted
+ // after trying to emit vtable speculatively. Because of this we do
+ // an extra pass after emitting all deferred vtables to find and emit
+ // these vtables opportunistically.
+ if (!Entry || Entry->isDeclaration())
+ return true;
+ }
+ retur
