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(CGM.GetAddrOfGlobal(TargetDecl)); @@ -3793,6 +3794,49 @@ // 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(MD); + auto *DD = CD ? nullptr : cast(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( + CGM.getModule().getNamedValue(CGM.getMangledName(BaseDecl))); + auto *Complete = cast_or_null( + CGM.getModule().getNamedValue(CGM.getMangledName(CompleteDecl))); + auto *Deleting = + DeletingDecl.getDecl() + ? cast_or_null( + 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, @@ -3801,6 +3845,11 @@ const CXXDestructorDecl *DD = CD ? nullptr : cast(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) { 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,12 +50,18 @@ } 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 struct foobar { foobar() { g(); } @@ -50,13 +69,33 @@ foobar 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 struct foobar { + ~foobar() { g(); } +}; +foobar 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,7 +182,7 @@ }; } 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 } @@ -129,6 +191,11 @@ // 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 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: 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