[CodeGen] Disable aggressive structor optimizations at -O0, take 2

The first version of the patch (r332228) was flawed because it was
putting structors into C5/D5 comdats very eagerly. This is correct only
if we can ensure the comdat contains all required versions of the
structor (which wasn't the case). This version uses a more nuanced
approach:
- for local structor symbols we use an alias because we don't have to
  worry about comdats or other compilation units.
- linkonce symbols are emitted separately, as we cannot guarantee we
  will have all symbols we need to form a comdat (they are emitted
  lazily, only when referenced).
- available_externally symbols are also emitted separately, as the code
  seemed to be worried about emitting an alias in this case.
- other linkage types are not affected by the optimization level. They
  either get put into a comdat (weak) or get aliased (external).

Reviewers: rjmccall, aprantl

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D46685

llvm-svn: 332839

3 files changed, 49 insertions, 14 deletions

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index c77644b56a5..0374e4cf9ac 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -3628,12 +3628,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/clang/test/CodeGenCXX/ctor-dtor-alias.cpp b/clang/test/CodeGenCXX/ctor-dtor-alias.cpp
index d72c5b48b3a..b44387a0341 100644
--- a/clang/test/CodeGenCXX/ctor-dtor-alias.cpp
+++ b/clang/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 alias void {{.*}} @_ZN5test16foobarIvEC2Ev
+// NOOPT1: @_ZN5test16foobarIvED1Ev = weak_odr 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<void>;
 }
 
 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 <typename T> 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 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 = alias void {{.*}} @_ZN5test71BD2Ev
+  // NOOPT3: define void @_ZN5test71BD2Ev
   template <typename> struct A {
     ~A() {}
   };
diff --git a/clang/test/CodeGenCXX/float16-declarations.cpp b/clang/test/CodeGenCXX/float16-declarations.cpp
index e82c05ec8c8..1d6999fef07 100644
--- a/clang/test/CodeGenCXX/float16-declarations.cpp
+++ b/clang/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