r315656 - [MS] Don't bail on replacing dllimport vbase dtors with base dtors
Reid Kleckner via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 12 17:53:02 PDT 2017
Author: rnk
Date: Thu Oct 12 17:53:02 2017
New Revision: 315656
URL: http://llvm.org/viewvc/llvm-project?rev=315656&view=rev
Log:
[MS] Don't bail on replacing dllimport vbase dtors with base dtors
Fix PR32990 by effectively reverting r283063 and solving it a different
way.
We want to limit the hack to not replace equivalent available_externally
dtors specifically to libc++, which uses always_inline. It seems certain
versions of libc++ do not provide all the symbols that an explicit
template instantiation is expected to provide.
If we get to the code that forms a real alias, only *then* check if this
is available_externally, and do that by asking a better question, which
is "is this a declaration for the linker?", because *that's* what means
we can't form an alias to it.
As a follow-on simplification, remove the InEveryTU parameter. Its last
use guarded this code for forming aliases, but we should never form
aliases to declarations, regardless of what we know about every TU.
Added:
cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
Modified:
cfe/trunk/lib/CodeGen/CGCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
Modified: cfe/trunk/lib/CodeGen/CGCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXX.cpp?rev=315656&r1=315655&r2=315656&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCXX.cpp Thu Oct 12 17:53:02 2017
@@ -110,16 +110,14 @@ bool CodeGenModule::TryEmitBaseDestructo
return true;
return TryEmitDefinitionAsAlias(GlobalDecl(D, Dtor_Base),
- GlobalDecl(BaseD, Dtor_Base),
- false);
+ GlobalDecl(BaseD, Dtor_Base));
}
/// Try to emit a definition as a global alias for another definition.
/// If \p InEveryTU is true, we know that an equivalent alias can be produced
/// in every translation unit.
bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
- GlobalDecl TargetDecl,
- bool InEveryTU) {
+ GlobalDecl TargetDecl) {
if (!getCodeGenOpts().CXXCtorDtorAliases)
return true;
@@ -134,11 +132,6 @@ bool CodeGenModule::TryEmitDefinitionAsA
llvm::GlobalValue::LinkageTypes TargetLinkage =
getFunctionLinkage(TargetDecl);
- // available_externally definitions aren't real definitions, so we cannot
- // create an alias to one.
- if (TargetLinkage == llvm::GlobalValue::AvailableExternallyLinkage)
- return true;
-
// Check if we have it already.
StringRef MangledName = getMangledName(AliasDecl);
llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
@@ -161,7 +154,14 @@ bool CodeGenModule::TryEmitDefinitionAsA
// Instead of creating as alias to a linkonce_odr, replace all of the uses
// of the aliasee.
- if (llvm::GlobalValue::isDiscardableIfUnused(Linkage)) {
+ if (llvm::GlobalValue::isDiscardableIfUnused(Linkage) &&
+ !(TargetLinkage == llvm::GlobalValue::AvailableExternallyLinkage &&
+ TargetDecl.getDecl()->hasAttr<AlwaysInlineAttr>())) {
+ // FIXME: An extern template instantiation will create functions with
+ // linkage "AvailableExternally". In libc++, some classes also define
+ // members with attribute "AlwaysInline" and expect no reference to
+ // be generated. It is desirable to reenable this optimisation after
+ // corresponding LLVM changes.
addReplacement(MangledName, Aliasee);
return false;
}
@@ -176,13 +176,11 @@ bool CodeGenModule::TryEmitDefinitionAsA
return true;
}
- if (!InEveryTU) {
- // If we don't have a definition for the destructor yet, don't
- // emit. We can't emit aliases to declarations; that's just not
- // how aliases work.
- if (Ref->isDeclaration())
- return true;
- }
+ // If we don't have a definition for the destructor yet or the definition is
+ // avaialable_externally, don't emit an alias. We can't emit aliases to
+ // declarations; that's just not how aliases work.
+ if (Ref->isDeclarationForLinker())
+ return true;
// Don't create an alias to a linker weak symbol. This avoids producing
// different COMDATs in different TUs. Another option would be to
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=315656&r1=315655&r2=315656&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Thu Oct 12 17:53:02 2017
@@ -1145,8 +1145,7 @@ public:
/// are emitted lazily.
void EmitGlobal(GlobalDecl D);
- bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
- bool InEveryTU);
+ bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target);
bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
/// Set attributes for a global definition.
Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=315656&r1=315655&r2=315656&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Thu Oct 12 17:53:02 2017
@@ -3805,7 +3805,7 @@ static void emitCXXDestructor(CodeGenMod
if (!dtor->getParent()->getNumVBases() &&
(dtorType == StructorType::Complete || dtorType == StructorType::Base)) {
bool ProducedAlias = !CGM.TryEmitDefinitionAsAlias(
- GlobalDecl(dtor, Dtor_Complete), GlobalDecl(dtor, Dtor_Base), true);
+ GlobalDecl(dtor, Dtor_Complete), GlobalDecl(dtor, Dtor_Base));
if (ProducedAlias) {
if (dtorType == StructorType::Complete)
return;
Added: cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp?rev=315656&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp Thu Oct 12 17:53:02 2017
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc -fms-extensions -emit-llvm -o - | FileCheck %s
+
+// FIXME: We should really consider removing -mconstructor-aliases for MS C++
+// ABI. The risk of bugs introducing ABI incompatibility under
+// -mno-constructor-aliases is too high.
+
+// PR32990
+
+// Introduces the virtual destructor. We should use the base destructor
+// directly, no thunk needed.
+struct __declspec(dllimport) ImportIntroVDtor {
+ virtual ~ImportIntroVDtor() {}
+};
+
+struct BaseClass {
+ virtual ~BaseClass() {}
+};
+
+// Non-virtually inherits from a non-dllimport base class. We should again call
+// the derived base constructor directly. No need for the complete (aka vbase)
+// destructor.
+struct __declspec(dllimport) ImportOverrideVDtor : public BaseClass {
+ virtual ~ImportOverrideVDtor() {}
+};
+
+// Virtually inherits from a non-dllimport base class. This time we need to call
+// the complete destructor and emit it inline. It's not exported from the DLL,
+// and it must be emitted.
+struct __declspec(dllimport) ImportVBaseOverrideVDtor
+ : public virtual BaseClass {
+ virtual ~ImportVBaseOverrideVDtor() {}
+};
+
+extern "C" void testit() {
+ ImportIntroVDtor t1;
+ ImportOverrideVDtor t2;
+ ImportVBaseOverrideVDtor t3;
+}
+
+// The destructors are called in reverse order of construction. Only the third
+// needs the complete destructor (_D).
+// CHECK-LABEL: define void @testit()
+// CHECK: call void @"\01??_DImportVBaseOverrideVDtor@@QEAAXXZ"(%struct.ImportVBaseOverrideVDtor* %{{.*}})
+// CHECK: call void @"\01??1ImportOverrideVDtor@@UEAA at XZ"(%struct.ImportOverrideVDtor* %{{.*}})
+// CHECK: call void @"\01??1ImportIntroVDtor@@UEAA at XZ"(%struct.ImportIntroVDtor* %{{.*}})
+
+// CHECK-LABEL: define linkonce_odr void @"\01??_DImportVBaseOverrideVDtor@@QEAAXXZ"
+// CHECK-LABEL: declare dllimport void @"\01??1ImportOverrideVDtor@@UEAA at XZ"
+// CHECK-LABEL: declare dllimport void @"\01??1ImportIntroVDtor@@UEAA at XZ"
More information about the cfe-commits
mailing list