From 011d8f83ca5a93b4586fa401837a7f49ee8ad4dd Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 19 Jul 2018 14:05:22 +0000 Subject: [PATCH] [CodeGen] Disable aggressive structor optimizations at -O0, take 3 The previous version of this patch (r332839) was reverted because it was causing "definition with same mangled name as another definition" errors in some module builds. This was caused by an unrelated bug in module importing which it exposed. The importing problem was fixed in r336240, so this recommits the original patch (r332839). Differential Revision: https://reviews.llvm.org/D46685 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@337456 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/ItaniumCXXABI.cpp | 18 ++++++++--- test/CodeGenCXX/ctor-dtor-alias.cpp | 41 +++++++++++++++++++----- test/CodeGenCXX/float16-declarations.cpp | 2 +- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index 8dd94e4955..d156ffab56 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/lib/CodeGen/ItaniumCXXABI.cpp @@ -3737,12 +3737,22 @@ static StructorCodegen getCodegenToUse(CodeGenModule &CGM, } 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. + if (llvm::GlobalValue::isLocalLinkage(Linkage)) + return CGM.getCodeGenOpts().OptimizationLevel > 0 ? StructorCodegen::RAUW + : StructorCodegen::Alias; + // Linkonce structors cannot be aliased nor placed in a comdat, so these need + // to be emitted separately. // FIXME: Should we allow available_externally aliases? - if (!llvm::GlobalAlias::isValidLinkage(Linkage)) - return StructorCodegen::RAUW; + if (llvm::GlobalValue::isDiscardableIfUnused(Linkage) || + !llvm::GlobalAlias::isValidLinkage(Linkage)) + return CGM.getCodeGenOpts().OptimizationLevel > 0 ? StructorCodegen::RAUW + : StructorCodegen::Emit; if (llvm::GlobalValue::isWeakForLinker(Linkage)) { // Only ELF and wasm support COMDATs with arbitrary names (C5/D5). diff --git a/test/CodeGenCXX/ctor-dtor-alias.cpp b/test/CodeGenCXX/ctor-dtor-alias.cpp index 89244cc105..4715111a14 100644 --- a/test/CodeGenCXX/ctor-dtor-alias.cpp +++ b/test/CodeGenCXX/ctor-dtor-alias.cpp @@ -1,5 +1,7 @@ -// 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 --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 +23,13 @@ namespace test1 { // 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 +46,17 @@ template struct foobar; } 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 should still emit the complete constructor. +// NOOPT1: define internal void @__cxx_global_var_init() +// NOOPT1: call void @_ZN5test26foobarIvEC1Ev +// NOOPT1: define linkonce_odr void @_ZN5test26foobarIvEC1Ev({{.*}} comdat align void g(); template struct foobar { foobar() { g(); } @@ -57,6 +71,11 @@ namespace test3 { // CHECK1: define internal void @__cxx_global_var_init.1() // 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. +// NOOPT2: _ZN5test312_GLOBAL__N_11BD1Ev = internal unnamed_addr alias void {{.*}} @_ZN5test312_GLOBAL__N_11BD2Ev +// NOOPT2: define internal void @__cxx_global_var_init.1() +// NOOPT2: call i32 @__cxa_atexit{{.*}}_ZN5test312_GLOBAL__N_11BD1Ev namespace { struct A { ~A() {} @@ -77,11 +96,12 @@ namespace test4 { // 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. This enables the debugger to see both + // destructors. + // NOOPT2: define internal void @__cxx_global_var_init.2() + // NOOPT2: call i32 @__cxa_atexit{{.*}}@_ZN5test41BD1Ev + // NOOPT2: define linkonce_odr void @_ZN5test41BD1Ev({{.*}} comdat align struct A { virtual ~A() {} }; @@ -129,6 +149,11 @@ namespace test7 { // 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. + // NOOPT3: @_ZN5test71BD1Ev = unnamed_addr alias void {{.*}} @_ZN5test71BD2Ev + // NOOPT3: define void @_ZN5test71BD2Ev template struct A { ~A() {} }; diff --git a/test/CodeGenCXX/float16-declarations.cpp b/test/CodeGenCXX/float16-declarations.cpp index e82c05ec8c..1d6999fef0 100644 --- a/test/CodeGenCXX/float16-declarations.cpp +++ b/test/CodeGenCXX/float16-declarations.cpp @@ -103,7 +103,7 @@ int main(void) { 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 -- 2.50.1