labath created this revision.
Herald added subscribers: cfe-commits, aheejin.
This version of the patch attempts to mitigate the code size regressions
caused by the previous versions of this patch
(https://reviews.llvm.org/D46685). It does this by
using aliases and C5/https://reviews.llvm.org/D5 comdats more aggressively.
This is not completely trivial, as linkonce_odr structors (these are
othe ones that were previously causing the blowup) are emitte lazily,
only when they are referenced, but we are only able to use the comdat if
all of the elements that should go into it are emitted. This is
particularly tricky for virtual desctructors as here the comdat must
include all three versions (complete, base, deleting).
As we do not (?) want this to impact the decisions on whether to emit
certain variants or not (and the full set of emitted versions is not
easily accessible from the place where we make the comdat decision), we
have to opportunistically wait for all required versions to be emitted.
If they are, we coalesce them into a comdat.
Repository:
rC Clang
https://reviews.llvm.org/D49989
Files:
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/ctor-dtor-alias.cpp
test/CodeGenCXX/float16-declarations.cpp
Index: test/CodeGenCXX/float16-declarations.cpp
===================================================================
--- test/CodeGenCXX/float16-declarations.cpp
+++ test/CodeGenCXX/float16-declarations.cpp
@@ -103,7 +103,7 @@
C1 c1(f1l);
// CHECK-DAG: [[F1L:%[a-z0-9]+]] = load half, half* %{{.*}}, align 2
-// CHECK-DAG: call void @_ZN2C1C2EDF16_(%class.C1* %{{.*}}, half %{{.*}})
+// CHECK-DAG: call void @_ZN2C1C1EDF16_(%class.C1* %{{.*}}, half %{{.*}})
S1<_Float16> s1 = { 132.f16 };
// CHECK-DAG: @_ZZ4mainE2s1 = private unnamed_addr constant %struct.S1 { half 0xH5820 }, align 2
Index: test/CodeGenCXX/ctor-dtor-alias.cpp
===================================================================
--- test/CodeGenCXX/ctor-dtor-alias.cpp
+++ test/CodeGenCXX/ctor-dtor-alias.cpp
@@ -1,5 +1,11 @@
-// RUN: %clang_cc1 %s -triple i686-linux -emit-llvm -o - -mconstructor-aliases | FileCheck --check-prefix=NOOPT %s
-
+// RUN: %clang_cc1 %s -triple i686-linux -emit-llvm -o - -mconstructor-aliases > %t
+// RUN: FileCheck --check-prefix=NOOPT1 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT2 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT3 --implicit-check-not=_ZN6test2a6foobarIvED0Ev --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT4 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT5 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT6 --implicit-check-not=_ZN6test4a1BD0Ev --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT7 --input-file=%t %s
// RUN: %clang_cc1 %s -triple i686-linux -emit-llvm -o - -mconstructor-aliases -O1 -disable-llvm-passes > %t
// RUN: FileCheck --check-prefix=CHECK1 --input-file=%t %s
// RUN: FileCheck --check-prefix=CHECK2 --input-file=%t %s
@@ -21,6 +27,13 @@
// CHECK1: define weak_odr void @_ZN5test16foobarIvED0Ev({{.*}} comdat($_ZN5test16foobarIvED5Ev)
// CHECK1-NOT: comdat
+// This should happen regardless of the opt level.
+// NOOPT1: @_ZN5test16foobarIvEC1Ev = weak_odr unnamed_addr alias void {{.*}} @_ZN5test16foobarIvEC2Ev
+// NOOPT1: @_ZN5test16foobarIvED1Ev = weak_odr unnamed_addr alias void (%"struct.test1::foobar"*), void (%"struct.test1::foobar"*)* @_ZN5test16foobarIvED2Ev
+// NOOPT1: define weak_odr void @_ZN5test16foobarIvEC2Ev({{.*}} comdat($_ZN5test16foobarIvEC5Ev)
+// NOOPT1: define weak_odr void @_ZN5test16foobarIvED2Ev({{.*}} comdat($_ZN5test16foobarIvED5Ev)
+// NOOPT1: define weak_odr void @_ZN5test16foobarIvED0Ev({{.*}} comdat($_ZN5test16foobarIvED5Ev)
+
// COFF doesn't support comdats with arbitrary names (C5/D5).
// COFF: define weak_odr {{.*}} void @_ZN5test16foobarIvEC2Ev({{.*}} comdat align
// COFF: define weak_odr {{.*}} void @_ZN5test16foobarIvEC1Ev({{.*}} comdat align
@@ -37,26 +50,52 @@
}
namespace test2 {
-// test that when the destrucor is linkonce_odr we just replace every use of
+// test that when the destructor is linkonce_odr we just replace every use of
// C1 with C2.
// CHECK1: define internal void @__cxx_global_var_init()
// CHECK1: call void @_ZN5test26foobarIvEC2Ev
// CHECK1: define linkonce_odr void @_ZN5test26foobarIvEC2Ev({{.*}} comdat align
+
+// At -O0, we still emit both symbols, but C1 is an alias to save space.
+// NOOPT2: @_ZN5test26foobarIvEC1Ev = linkonce_odr unnamed_addr alias void {{.*}} @_ZN5test26foobarIvEC2Ev
+// NOOPT2: define internal void @__cxx_global_var_init()
+// NOOPT2: call void @_ZN5test26foobarIvEC1Ev
+// NOOPT2: define linkonce_odr void @_ZN5test26foobarIvEC2Ev({{.*}} comdat($_ZN5test26foobarIvEC5Ev) align
void g();
template <typename T> struct foobar {
foobar() { g(); }
};
foobar<void> x;
}
+namespace test2a {
+// Similar to test2, but check destructors. As D0 is not referenced and not
+// virtual, it shouldn't be emitted, nor placed into a comdat (at -O0).
+
+void g();
+template <typename T> struct foobar {
+ ~foobar() { g(); }
+};
+foobar<void> x;
+// NOOPT3: @_ZN6test2a6foobarIvED1Ev = linkonce_odr unnamed_addr alias void {{.*}} @_ZN6test2a6foobarIvED2Ev
+// NOOPT3: define internal void @__cxx_global_var_init.1()
+// NOOPT3: call i32 @__cxa_atexit{{.*}} @_ZN6test2a6foobarIvED1Ev
+// NOOPT3: define linkonce_odr void @_ZN6test2a6foobarIvED2Ev({{.*}} comdat($_ZN6test2a6foobarIvED5Ev) align
+}
+
namespace test3 {
// test that instead of an internal alias we just use the other destructor
// directly.
-// CHECK1: define internal void @__cxx_global_var_init.1()
+// CHECK1: define internal void @__cxx_global_var_init.2()
// CHECK1: call i32 @__cxa_atexit{{.*}}_ZN5test312_GLOBAL__N_11AD2Ev
// CHECK1: define internal void @_ZN5test312_GLOBAL__N_11AD2Ev(
+
+// We can use an alias for internal symbols at -O0.
+// NOOPT4: _ZN5test312_GLOBAL__N_11BD1Ev = internal unnamed_addr alias void {{.*}} @_ZN5test312_GLOBAL__N_11BD2Ev
+// NOOPT4: define internal void @__cxx_global_var_init.2()
+// NOOPT4: call i32 @__cxa_atexit{{.*}}_ZN5test312_GLOBAL__N_11BD1Ev
namespace {
struct A {
~A() {}
@@ -73,15 +112,20 @@
// guarantee that they will be present in every TU. Instead, we just call
// A's destructor directly.
- // CHECK1: define internal void @__cxx_global_var_init.2()
+ // CHECK1: define internal void @__cxx_global_var_init.3()
// CHECK1: call i32 @__cxa_atexit{{.*}}_ZN5test41AD2Ev
// CHECK1: define linkonce_odr void @_ZN5test41AD2Ev({{.*}} comdat align
- // 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_init.2()
- // NOOPT: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD2Ev
- // NOOPT: define linkonce_odr void @_ZN5test41BD2Ev({{.*}} comdat align
+ // Test that we don't do this optimization at -O0 and call the complete
+ // destructor for B instead. The complete destructor is an alias to the base
+ // one. Because all three destructor flavours are emitted, they get put into a
+ // D5 comdat.
+ // NOOPT5: @_ZTVN5test41BE = linkonce_odr unnamed_addr constant { [4 x i8*] } { {{.*}} null, {{.*}} @_ZTIN5test41BE {{.*}} @_ZN5test41BD1Ev {{.*}} @_ZN5test41BD0Ev
+ // NOOPT5: _ZN5test41BD1Ev = linkonce_odr unnamed_addr alias void {{.*}} @_ZN5test41BD2Ev
+ // NOOPT5: define internal void @__cxx_global_var_init.3()
+ // NOOPT5: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD1Ev
+ // NOOPT5: define linkonce_odr void @_ZN5test41BD0Ev({{.*}} comdat($_ZN5test41BD5Ev) align
+ // NOOPT5: define linkonce_odr void @_ZN5test41BD2Ev({{.*}} comdat($_ZN5test41BD5Ev) align
struct A {
virtual ~A() {}
};
@@ -91,10 +135,28 @@
B X;
}
+namespace test4a {
+ // Similar to test4, but B's vtable is not emitted (in this CU). As this
+ // causes D0 to not be emitted, D1 and D2 cannot be aliases in a single
+ // comdat anymore.
+ struct A {
+ virtual ~A() {}
+ };
+ struct B : public A {
+ ~B() {}
+ virtual void anchor();
+ };
+ B X;
+ // NOOPT6: define internal void @__cxx_global_var_init.3()
+ // NOOPT6: call i32 @__cxa_atexit{{.*}}@_ZN6test4a1BD1Ev
+ // NOOPT6: define linkonce_odr void @_ZN6test4a1BD1Ev({{.*}} comdat align
+ // NOOPT6: define linkonce_odr void @_ZN6test4a1BD2Ev({{.*}} comdat align
+}
+
namespace test5 {
// similar to test4, but with an internal B.
- // CHECK2: define internal void @__cxx_global_var_init.3()
+ // CHECK2: define internal void @__cxx_global_var_init.4()
// CHECK2: call i32 @__cxa_atexit{{.*}}_ZN5test51AD2Ev
// CHECK2: define linkonce_odr void @_ZN5test51AD2Ev({{.*}} comdat align
struct A {
@@ -120,15 +182,20 @@
};
}
B X;
- // CHECK3: define internal void @__cxx_global_var_init.4()
+ // CHECK3: define internal void @__cxx_global_var_init.5()
// CHECK3: call i32 @__cxa_atexit({{.*}}@_ZN5test61AD2Ev
}
namespace test7 {
// Test that we don't produce an alias from ~B to ~A<int> (or crash figuring
// out if we should).
// pr17875.
// CHECK3: define void @_ZN5test71BD2Ev
+
+ // At -O0, we should emit both destructors, the complete can be an alias to
+ // the base one.
+ // NOOPT7: @_ZN5test71BD1Ev = unnamed_addr alias void {{.*}} @_ZN5test71BD2Ev
+ // NOOPT7: define void @_ZN5test71BD2Ev
template <typename> struct A {
~A() {}
};
@@ -142,7 +209,7 @@
namespace test8 {
// Test that we replace ~zed with ~bar which is an alias to ~foo.
// CHECK4: @_ZN5test83barD2Ev = unnamed_addr alias {{.*}} @_ZN5test83fooD2Ev
- // CHECK4: define internal void @__cxx_global_var_init.5()
+ // CHECK4: define internal void @__cxx_global_var_init.6()
// CHECK4: call i32 @__cxa_atexit({{.*}}@_ZN5test83barD2Ev
struct foo {
~foo();
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -3744,33 +3744,34 @@
}
llvm::GlobalValue::LinkageTypes Linkage = CGM.getFunctionLinkage(AliasDecl);
- if (llvm::GlobalValue::isDiscardableIfUnused(Linkage))
- return StructorCodegen::RAUW;
-
+ // All discardable structors can be RAUWed, but we don't want to do that in
+ // unoptimized code, as that makes complete structor symbol disappear
+ // completely, which degrades debugging experience.
+ // Symbols with private linkage can be safely aliased, so we special case them
+ // here.
// FIXME: Should we allow available_externally aliases?
- if (!llvm::GlobalAlias::isValidLinkage(Linkage))
- return StructorCodegen::RAUW;
-
- if (llvm::GlobalValue::isWeakForLinker(Linkage)) {
- // Only ELF and wasm support COMDATs with arbitrary names (C5/D5).
- if (CGM.getTarget().getTriple().isOSBinFormatELF() ||
- CGM.getTarget().getTriple().isOSBinFormatWasm())
- return StructorCodegen::COMDAT;
- return StructorCodegen::Emit;
- }
+ if (llvm::GlobalValue::isLocalLinkage(Linkage))
+ return CGM.getCodeGenOpts().OptimizationLevel > 0 ? StructorCodegen::RAUW
+ : StructorCodegen::Alias;
+
+ if (llvm::GlobalValue::isDiscardableIfUnused(Linkage) ||
+ !llvm::GlobalAlias::isValidLinkage(Linkage))
+ return CGM.getCodeGenOpts().OptimizationLevel > 0 ? StructorCodegen::RAUW
+ : StructorCodegen::COMDAT;
+
+ if (llvm::GlobalValue::isWeakForLinker(Linkage))
+ return StructorCodegen::COMDAT;
return StructorCodegen::Alias;
}
-static void emitConstructorDestructorAlias(CodeGenModule &CGM,
- GlobalDecl AliasDecl,
- GlobalDecl TargetDecl) {
+static llvm::GlobalAlias *
+emitConstructorDestructorAlias(CodeGenModule &CGM, GlobalDecl AliasDecl,
+ GlobalDecl TargetDecl) {
llvm::GlobalValue::LinkageTypes Linkage = CGM.getFunctionLinkage(AliasDecl);
StringRef MangledName = CGM.getMangledName(AliasDecl);
llvm::GlobalValue *Entry = CGM.GetGlobalValue(MangledName);
- if (Entry && !Entry->isDeclaration())
- return;
auto *Aliasee = cast<llvm::GlobalValue>(CGM.GetAddrOfGlobal(TargetDecl));
@@ -3793,14 +3794,62 @@
// Finally, set up the alias with its proper name and attributes.
CGM.SetCommonAttributes(AliasDecl, Alias);
+ return Alias;
+}
+
+static void fixupComdat(CodeGenModule &CGM, const CXXMethodDecl *MD,
+ StructorType Type, ItaniumMangleContext &MC) {
+ auto *CD = dyn_cast<CXXConstructorDecl>(MD);
+ auto *DD = CD ? nullptr : cast<CXXDestructorDecl>(MD);
+
+ GlobalDecl BaseDecl =
+ CD ? GlobalDecl(CD, Ctor_Base) : GlobalDecl(DD, Dtor_Base);
+ GlobalDecl CompleteDecl =
+ CD ? GlobalDecl(CD, Ctor_Complete) : GlobalDecl(DD, Dtor_Complete);
+ GlobalDecl DeletingDecl = DD ? GlobalDecl(DD, Dtor_Deleting) : GlobalDecl();
+
+ auto *Base = cast_or_null<llvm::GlobalObject>(
+ CGM.getModule().getNamedValue(CGM.getMangledName(BaseDecl)));
+ auto *Complete = cast_or_null<llvm::GlobalObject>(
+ CGM.getModule().getNamedValue(CGM.getMangledName(CompleteDecl)));
+ auto *Deleting =
+ DeletingDecl.getDecl()
+ ? cast_or_null<llvm::GlobalObject>(
+ CGM.getModule().getNamedValue(CGM.getMangledName(DeletingDecl)))
+ : nullptr;
+
+ if (!Base || Base->isDeclaration())
+ return;
+ if (!Complete || Complete->isDeclaration())
+ return;
+ if (DD && DD->isVirtual() && (!Deleting || Deleting->isDeclaration()))
+ return;
+
+ emitConstructorDestructorAlias(CGM, CompleteDecl, BaseDecl);
+
+ SmallString<256> Buffer;
+ llvm::raw_svector_ostream Out(Buffer);
+ if (DD)
+ MC.mangleCXXDtorComdat(DD, Out);
+ else
+ MC.mangleCXXCtorComdat(CD, Out);
+ llvm::Comdat *C = CGM.getModule().getOrInsertComdat(Out.str());
+ Base->setComdat(C);
+ if (DD && DD->isVirtual())
+ Deleting->setComdat(C);
}
void ItaniumCXXABI::emitCXXStructor(const CXXMethodDecl *MD,
StructorType Type) {
auto *CD = dyn_cast<CXXConstructorDecl>(MD);
const CXXDestructorDecl *DD = CD ? nullptr : cast<CXXDestructorDecl>(MD);
StructorCodegen CGType = getCodegenToUse(CGM, MD);
+ // Only ELF and wasm support COMDATs with arbitrary names (C5/D5).
+ if (CGType == StructorCodegen::COMDAT &&
+ !CGM.getTarget().getTriple().isOSBinFormatELF() &&
+ !CGM.getTarget().getTriple().isOSBinFormatWasm())
+ CGType = StructorCodegen::Emit;
if (Type == StructorType::Complete) {
GlobalDecl CompleteDecl;
@@ -3813,7 +3862,7 @@
BaseDecl = GlobalDecl(DD, Dtor_Base);
}
- if (CGType == StructorCodegen::Alias || CGType == StructorCodegen::COMDAT) {
+ if (CGType == StructorCodegen::Alias) {
emitConstructorDestructorAlias(CGM, CompleteDecl, BaseDecl);
return;
}
@@ -3848,18 +3897,9 @@
llvm::Function *Fn = CGM.codegenCXXStructor(MD, Type);
- if (CGType == StructorCodegen::COMDAT) {
- SmallString<256> Buffer;
- llvm::raw_svector_ostream Out(Buffer);
- if (DD)
- getMangleContext().mangleCXXDtorComdat(DD, Out);
- else
- getMangleContext().mangleCXXCtorComdat(CD, Out);
- llvm::Comdat *C = CGM.getModule().getOrInsertComdat(Out.str());
- Fn->setComdat(C);
- } else {
- CGM.maybeSetTrivialComdat(*MD, *Fn);
- }
+ CGM.maybeSetTrivialComdat(*MD, *Fn);
+ if (CGType == StructorCodegen::COMDAT)
+ fixupComdat(CGM, MD, Type, getMangleContext());
}
static llvm::Constant *getBeginCatchFn(CodeGenModule &CGM) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
