The attached patch makes clang use comdats for linkonce_odr structors.
This matches the gcc behavior and there are two potential advantages
that I can think off
* Avoid a bit of code bloat when mixing gcc and clang compiled code
since they will have the same COMDATs.
* Avoid pain to users that developed a dependency on the gcc behavior,
but it might be a bit late for that
(http://llvm.org/devmtg/2014-04/PDFs/Talks/NickRefactoring.pdf).
I was hoping this wouldn't have a noticeable code quality impact, but
I have seen at least two issues
* Putting D0 in the D5 comdat is an ABI bug IMHO since it is never
equal to D1 or D2 and doing so with linkonce_odr can bloat the code a
bit by causing us to keep more functions then we need (keep D1/D2 if
they get inlined into D0 for example).
* Using a COMDAT for the destructors of a class bar prevents D2 from
being replaced with D2 of the base class. That means we end up with
define linkonce_odr void @_ZN3fooIiED2Ev(%struct.foo* %this)
unnamed_addr #1 comdat $_ZN3fooIiED5Ev align 2 {
entry:
%0 = bitcast %struct.foo* %this to %struct.bar*
tail call void @_ZN3barD2Ev(%struct.bar* %0)
ret void
}
Instead of rauw of _ZN3fooIiED2Ev with _ZN3barD2Ev. We could teach
LLVM to do this, but we are not there yet.
So it is not clear if this is worth it having it upstream. What do you
guys think?
Cheers,
Rafael
diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h
index 955f665..4ff6a9c 100644
--- a/lib/CodeGen/CodeGenModule.h
+++ b/lib/CodeGen/CodeGenModule.h
@@ -320,7 +320,7 @@ class CodeGenModule : public CodeGenTypeCache {
/// referenced. These get code generated when the module is done.
struct DeferredGlobal {
DeferredGlobal(llvm::GlobalValue *GV, GlobalDecl GD) : GV(GV), GD(GD) {}
- llvm::AssertingVH<llvm::GlobalValue> GV;
+ llvm::TrackingVH<llvm::GlobalValue> GV;
GlobalDecl GD;
};
std::vector<DeferredGlobal> DeferredDeclsToEmit;
diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index 824eba9..5a9ede9 100644
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -3054,7 +3054,7 @@ static StructorCodegen getCodegenToUse(CodeGenModule &CGM,
}
llvm::GlobalValue::LinkageTypes Linkage = CGM.getFunctionLinkage(AliasDecl);
- if (llvm::GlobalValue::isDiscardableIfUnused(Linkage))
+ if (llvm::GlobalValue::isLocalLinkage(Linkage))
return StructorCodegen::RAUW;
// FIXME: Should we allow available_externally aliases?
@@ -3145,6 +3145,15 @@ void ItaniumCXXABI::emitCXXStructor(const CXXMethodDecl *MD,
llvm::Function *Fn = CGM.codegenCXXStructor(MD, Type);
if (CGType == StructorCodegen::COMDAT) {
+ // Make sure we also emit the complete and deleting ones.
+ if (Type == StructorType::Base && Fn->hasLinkOnceLinkage()) {
+ CGM.getAddrOfCXXStructor(MD, StructorType::Complete);
+ if (auto *D = dyn_cast<CXXDestructorDecl>(MD)) {
+ if (D->isVirtual())
+ CGM.getAddrOfCXXStructor(MD, StructorType::Deleting);
+ }
+ }
+
SmallString<256> Buffer;
llvm::raw_svector_ostream Out(Buffer);
if (DD)
diff --git a/test/CodeGenCXX/ctor-dtor-alias.cpp b/test/CodeGenCXX/ctor-dtor-alias.cpp
index 1ae9ead..012f70b 100644
--- a/test/CodeGenCXX/ctor-dtor-alias.cpp
+++ b/test/CodeGenCXX/ctor-dtor-alias.cpp
@@ -7,6 +7,8 @@
// RUN: FileCheck --check-prefix=CHECK4 --input-file=%t %s
// RUN: FileCheck --check-prefix=CHECK5 --input-file=%t %s
// RUN: FileCheck --check-prefix=CHECK6 --input-file=%t %s
+// RUN: FileCheck --check-prefix=CHECK7 --input-file=%t %s
+// RUN: FileCheck --check-prefix=CHECK8 --input-file=%t %s
// RUN: %clang_cc1 %s -triple i686-pc-windows-gnu -emit-llvm -o - -mconstructor-aliases -O1 -disable-llvm-optzns | FileCheck --check-prefix=COFF %s
@@ -19,7 +21,7 @@ namespace test1 {
// CHECK1: define weak_odr void @_ZN5test16foobarIvEC2Ev({{.*}} comdat $_ZN5test16foobarIvEC5Ev
// CHECK1: define weak_odr void @_ZN5test16foobarIvED2Ev({{.*}} comdat $_ZN5test16foobarIvED5Ev
// CHECK1: define weak_odr void @_ZN5test16foobarIvED0Ev({{.*}} comdat $_ZN5test16foobarIvED5Ev
-// CHECK1-NOT: comdat
+// CHECK1-NOT: _ZN5test1{{.*}}comdat
// COFF doesn't support comdats with arbitrary names (C5/D5).
// COFF-NOT: comdat
@@ -34,12 +36,12 @@ template struct foobar<void>;
}
namespace test2 {
-// test that when the destrucor is linkonce_odr we just replace every use of
-// C1 with C2.
+// test that ...
-// CHECK1: define internal void @__cxx_global_var_init()
-// CHECK1: call void @_ZN5test26foobarIvEC2Ev
-// CHECK1: define linkonce_odr void @_ZN5test26foobarIvEC2Ev(
+// CHECK2: _ZN5test26foobarIvEC1Ev = linkonce_odr alias {{.*}} @_ZN5test26foobarIvEC2Ev
+// CHECK2: define internal void @__cxx_global_var_init()
+// CHECK2: call void @_ZN5test26foobarIvEC1Ev
+// CHECK2: define linkonce_odr void @_ZN5test26foobarIvEC2Ev(
void g();
template <typename T> struct foobar {
foobar() { g(); }
@@ -51,9 +53,9 @@ namespace test3 {
// test that instead of an internal alias we just use the other destructor
// directly.
-// CHECK1: define internal void @__cxx_global_var_init1()
-// CHECK1: call i32 @__cxa_atexit{{.*}}_ZN5test312_GLOBAL__N_11AD2Ev
-// CHECK1: define internal void @_ZN5test312_GLOBAL__N_11AD2Ev(
+// CHECK3: define internal void @__cxx_global_var_init1()
+// CHECK3: call i32 @__cxa_atexit{{.*}}_ZN5test312_GLOBAL__N_11AD2Ev
+// CHECK3: define internal void @_ZN5test312_GLOBAL__N_11AD2Ev(
namespace {
struct A {
~A() {}
@@ -70,30 +72,32 @@ namespace test4 {
// guarantee that they will be present in every TU. Instead, we just call
// A's destructor directly.
- // CHECK1: define internal void @__cxx_global_var_init2()
- // CHECK1: call i32 @__cxa_atexit{{.*}}_ZN5test41AD2Ev
- // CHECK1: define linkonce_odr void @_ZN5test41AD2Ev(
+ // CHECK3: define internal void @__cxx_global_var_init2()
+ // CHECK3: call i32 @__cxa_atexit{{.*}}_ZN5test41AD2Ev
+ // CHECK3: define linkonce_odr void @_ZN5test41AD2Ev(
// test that we don't do this optimization at -O0 so that the debugger can
// see both destructors.
// NOOPT: define internal void @__cxx_global_var_init2()
- // NOOPT: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD2Ev
- // NOOPT: define linkonce_odr void @_ZN5test41BD2Ev
+ // NOOPT: call i32 @__cxa_atexit{{.*}}@_ZN5test412_GLOBAL__N_11BD2Ev
+ // NOOPT: define internal void @_ZN5test412_GLOBAL__N_11BD2Ev
struct A {
virtual ~A() {}
};
+ namespace {
struct B : public A{
~B() {}
};
+ }
B X;
}
namespace test5 {
// similar to test4, but with an internal B.
- // CHECK2: define internal void @__cxx_global_var_init3()
- // CHECK2: call i32 @__cxa_atexit{{.*}}_ZN5test51AD2Ev
- // CHECK2: define linkonce_odr void @_ZN5test51AD2Ev(
+ // CHECK3: define internal void @__cxx_global_var_init3()
+ // CHECK3: call i32 @__cxa_atexit{{.*}}_ZN5test51AD2Ev
+ // CHECK3: define linkonce_odr void @_ZN5test51AD2Ev(
struct A {
virtual ~A() {}
};
@@ -149,7 +153,9 @@ namespace test8 {
~bar();
};
bar::~bar() {}
+ namespace {
struct zed : public bar {};
+ }
zed foo;
}
@@ -159,17 +165,19 @@ struct foo {
}
};
+namespace {
struct bar : public foo {};
+}
void zed() {
// Test that we produce a call to bar's destructor. We used to call foo's, but
// it has a different calling conversion.
- // CHECK4: call void @_ZN5test93barD2Ev
+ // CHECK4: call void @_ZN5test912_GLOBAL__N_13barD2Ev
bar ptr;
}
}
-// CHECK5: @_ZTV1C = linkonce_odr unnamed_addr constant [4 x i8*] [{{[^@]*}}@_ZTI1C {{[^@]*}}@_ZN1CD2Ev {{[^@]*}}@_ZN1CD0Ev {{[^@]*}}]
+// CHECK5: @_ZTV1C = linkonce_odr unnamed_addr constant [4 x i8*] [{{[^@]*}}@_ZTI1C {{[^@]*}}@_ZN1CD1Ev {{[^@]*}}@_ZN1CD0Ev {{[^@]*}}]
// r194296 replaced C::~C with B::~B without emitting the later.
class A {
@@ -232,3 +240,42 @@ foo::~foo() {}
// CHECK6: @_ZN6test113fooD2Ev = alias {{.*}} @_ZN6test113barD2Ev
// CHECK6: @_ZN6test113fooD1Ev = alias {{.*}} @_ZN6test113fooD2Ev
}
+
+namespace test12 {
+// Test that we emit both the base and complete constructors of foo even if
+// only the base is used.
+// CHECK6: @_ZN6test123fooIiEC1Ev = linkonce_odr alias {{.*}} @_ZN6test123fooIiEC2Ev
+// CHECK6: define linkonce_odr void @_ZN6test123fooIiEC2Ev({{.*}} comdat $_ZN6test123fooIiEC5Ev
+template<typename T>
+struct foo {
+ foo() {}
+};
+
+struct bar : public foo<int> {
+ bar() {}
+};
+
+bar x;
+}
+
+namespace test13 {
+// CHECK7: @_ZN6test133fooC1Ev = linkonce_odr alias {{.*}} @_ZN6test133fooC2Ev
+// CHECK7: define linkonce_odr void @_ZN6test133fooC2Ev({{.*}} comdat $_ZN6test133fooC5Ev
+
+struct foo {
+ foo() {}
+};
+foo x;
+}
+
+namespace test14 {
+// Test that we produce D0 to satisfy the comdat requirements.
+// CHECK8: define linkonce_odr void @_ZN6test143optD0Ev({{.*}} comdat $_ZN6test143optD5Ev
+struct Option {
+ virtual ~Option();
+};
+struct opt : public Option {
+ opt();
+};
+opt InputFilename;
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
