WIP: [CodeGen] Disable aggressive structor optimizations at -O0, take 4

[labath]

Summary

This version of the patch attempts to mitigate the code size regressions
caused by the previous versions of this patch (D46685). It does this by
using aliases and C5/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.

lib/CodeGen/ItaniumCXXABI.cpp

static StructorCodegen getCodegenToUse(CodeGenModule &CGM,
   if (const auto *DD = dyn_cast<CXXDestructorDecl>(MD)) {
     AliasDecl = GlobalDecl(DD, Dtor_Complete);
   } else {
     const auto *CD = cast<CXXConstructorDecl>(MD);
     AliasDecl = GlobalDecl(CD, Ctor_Complete);
   }
   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::isLocalLinkage(Linkage))
+    return CGM.getCodeGenOpts().OptimizationLevel > 0 ? StructorCodegen::RAUW
+                                                      : StructorCodegen::Alias;
 
-  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::isDiscardableIfUnused(Linkage) ||
+      !llvm::GlobalAlias::isValidLinkage(Linkage))
+    return CGM.getCodeGenOpts().OptimizationLevel > 0 ? StructorCodegen::RAUW
+                                                      : StructorCodegen::COMDAT;
+
+  if (llvm::GlobalValue::isWeakForLinker(Linkage))
 
   return StructorCodegen::Alias;
 }
 
-static void emitConstructorDestructorAlias(CodeGenModule &CGM,
-                                           GlobalDecl AliasDecl,
-                                           GlobalDecl TargetDecl) {
+static llvm::GlobalAlias *
+emitConstructorDestructorAlias(CodeGenModule &CGM, GlobalDecl AliasDecl,
   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));
 
   // Create the alias with no name.
   auto *Alias = llvm::GlobalAlias::create(Linkage, "", Aliasee);
 
   // Constructors and destructors are always unnamed_addr.
   Alias->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 
   // Switch any previous uses to the alias.
   if (Entry) {
     assert(Entry->getType() == Aliasee->getType() &&
            "declaration exists with different type");
     Alias->takeName(Entry);
     Entry->replaceAllUsesWith(Alias);
     Entry->eraseFromParent();
   } else {
     Alias->setName(MangledName);
   }
 
   // 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;
     GlobalDecl BaseDecl;
     if (CD) {
       CompleteDecl = GlobalDecl(CD, Ctor_Complete);
       BaseDecl = GlobalDecl(CD, Ctor_Base);
     } else {
       CompleteDecl = GlobalDecl(DD, Dtor_Complete);
       BaseDecl = GlobalDecl(DD, Dtor_Base);
     }
 
-    if (CGType == StructorCodegen::Alias || CGType == StructorCodegen::COMDAT) {
+    if (CGType == StructorCodegen::Alias) {
       emitConstructorDestructorAlias(CGM, CompleteDecl, BaseDecl);
       return;
     }
 
     if (CGType == StructorCodegen::RAUW) {
       StringRef MangledName = CGM.getMangledName(CompleteDecl);
       auto *Aliasee = CGM.GetAddrOfGlobal(BaseDecl);
       CGM.addReplacement(MangledName, Aliasee);
void ItaniumCXXABI::emitCXXStructor(const CXXMethodDecl *MD,
   //  * the destructor does not have an implicit "this" return, and
   //  * the operator delete has the same calling convention and IR function type
   //    as the destructor.
   // In such cases we should try to emit the deleting dtor as an alias to the
   // selected 'operator delete'.
 
   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);
-  }
+  if (CGType == StructorCodegen::COMDAT)
+    fixupComdat(CGM, MD, Type, getMangleContext());
 }
 
 static llvm::Constant *getBeginCatchFn(CodeGenModule &CGM) {
   // void *__cxa_begin_catch(void*);
   llvm::FunctionType *FTy = llvm::FunctionType::get(
       CGM.Int8PtrTy, CGM.Int8PtrTy, /*IsVarArgs=*/false);
 
   return CGM.CreateRuntimeFunction(FTy, "__cxa_begin_catch");

test/CodeGenCXX/ctor-dtor-alias.cpp

-// 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
 // RUN: FileCheck --check-prefix=CHECK3 --input-file=%t %s
 // 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: %clang_cc1 %s -triple i686-pc-windows-gnu -emit-llvm -o - -mconstructor-aliases -O1 -disable-llvm-passes | FileCheck --check-prefix=COFF %s
 
 namespace test1 {
 // Test that we produce the appropriate comdats when creating aliases to
 // weak_odr constructors and destructors.
 
 // CHECK1: @_ZN5test16foobarIvEC1Ev = weak_odr unnamed_addr alias void {{.*}} @_ZN5test16foobarIvEC2Ev
 // CHECK1: @_ZN5test16foobarIvED1Ev = weak_odr unnamed_addr alias void (%"struct.test1::foobar"*), void (%"struct.test1::foobar"*)* @_ZN5test16foobarIvED2Ev
 // 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
 
+// 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
 // COFF: define weak_odr {{.*}} void @_ZN5test16foobarIvED2Ev({{.*}} comdat align
 // COFF: define weak_odr {{.*}} void @_ZN5test16foobarIvED0Ev({{.*}} comdat align
 
 template <typename T>
 struct foobar {
   foobar() {}
   virtual ~foobar() {}
 };
 
 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 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() {}
 };
 
 struct B : public A {};
 }
 
 B x;
 }
 
 namespace test4 {
   // Test that we don't produce aliases from B to A. We cannot because we cannot
   // 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() {}
   };
   struct B : public A{
     ~B() {}
   };
   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 {
     virtual ~A() {}
   };
   namespace {
   struct B : public A{
     ~B() {}
struct A {
     virtual ~A();
   };
   namespace {
   struct B : public A {
     ~B() {}
   };
   }
   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() {}
   };
   class B : A<int> {
     ~B();
   };
   template class A<int>;
   B::~B() {}
 }
 
 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();
   };
   foo::~foo() {}
   struct bar : public foo {
     ~bar();
   };

test/CodeGenCXX/float16-declarations.cpp

// CHECK-DAG: store half 0xH63D0, half* %{{.*}}, align 2
   _Float16 f2l = -0.f16;
 // CHECK-DAG: store half 0xH8000, half* %{{.*}}, align 2
 
   _Float16 f3l = 1.000976562;
 // CHECK-DAG: store half 0xH3C01, half* %{{.*}}, align 2
 
   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
 // CHECK-DAG:  [[S1:%[0-9]+]] = bitcast %struct.S1* %{{.*}} to i8*
 // CHECK-DAG: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 2 [[S1]], i8* align 2 bitcast (%struct.S1* @_ZZ4mainE2s1 to i8*), i64 2, i1 false)
 
   _Float16 f4l = func1n(f1l)  + func1f(f2l) + c1.func1c(f3l) + c1.func2c(f1l) +
     func1t(f1l) + s1.mem2 - f1n + f2n;