Skip to content

Commit

Permalink
Recommit [ABI] [C++20] [Modules] Don't generate vtable if the class i…
Browse files Browse the repository at this point in the history
…s defined in other module unit

Close #61940.

The root cause is that clang will generate vtable as strong symbol now
even if the corresponding class is defined in other module units. After
I check the wording in Itanium ABI, I find this is not inconsistent.
Itanium ABI 5.2.3
(https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) says:

> The virtual table for a class is emitted in the same object containing
> the definition of its key function, i.e. the first non-pure virtual
> function that is not inline at the point of class definition.

So the current behavior is incorrect. This patch tries to address this.
Also I think we need to do a similar change for MSVC ABI. But I don't
find the formal wording. So I don't address this in this patch.

Reviewed By: rjmccall, iains, dblaikie

Differential Revision: https://reviews.llvm.org/D150023
  • Loading branch information
ChuanqiXu9 committed Jun 19, 2023
1 parent b9a134a commit 2d8044e
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 1 deletion.
9 changes: 8 additions & 1 deletion clang/lib/CodeGen/CGVTables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1172,9 +1172,16 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
if (!keyFunction)
return false;

const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
return !keyFunction->hasBody();
if (!keyFunction->hasBody(Def))
return true;

assert(Def && "The body of the key function is not assigned to Def?");
// If the non-inline key function comes from another module unit, the vtable
// must be defined there.
return Def->isInAnotherModuleUnit() && !Def->isInlineSpecified();
}

/// Given that we're currently at the end of the translation unit, and
Expand Down
98 changes: 98 additions & 0 deletions clang/test/CodeGenCXX/modules-vtable.cppm
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// REQUIRES: !system-windows

// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: cd %t
//
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -emit-module-interface \
// RUN: %t/Mod.cppm -o %t/Mod.pcm
//
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/Mod.pcm \
// RUN: -emit-llvm -o - | FileCheck %t/Mod.cppm
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -fmodule-file=Mod=%t/Mod.pcm \
// RUN: %t/Use.cpp -emit-llvm -o - | FileCheck %t/Use.cpp
//
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -emit-module-interface \
// RUN: %t/Mod.cppm -o %t/Mod.pcm -DKEY_FUNCTION_INLINE
//
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/Mod.pcm \
// RUN: -emit-llvm -o - | FileCheck %t/Mod.cppm -check-prefix=CHECK-INLINE
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -fmodule-file=Mod=%t/Mod.pcm \
// RUN: %t/Use.cpp -emit-llvm -o - | FileCheck %t/Use.cpp -check-prefix=CHECK-INLINE
//
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -emit-module-interface \
// RUN: %t/M-A.cppm -o %t/M-A.pcm
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -fmodule-file=M:A=%t/M-A.pcm \
// RUN: %t/M-B.cppm -emit-llvm -o - | FileCheck %t/M-B.cppm

//--- Mod.cppm
export module Mod;

export class Base {
public:
virtual ~Base();
};
#ifdef KEY_FUNCTION_INLINE
inline
#endif
Base::~Base() {}

// CHECK: @_ZTVW3Mod4Base = unnamed_addr constant
// CHECK: @_ZTSW3Mod4Base = constant
// CHECK: @_ZTIW3Mod4Base = constant

// CHECK-INLINE: @_ZTVW3Mod4Base = linkonce_odr {{.*}}unnamed_addr constant
// CHECK-INLINE: @_ZTSW3Mod4Base = linkonce_odr {{.*}}constant
// CHECK-INLINE: @_ZTIW3Mod4Base = linkonce_odr {{.*}}constant

module :private;
int private_use() {
Base base;
return 43;
}

//--- Use.cpp
import Mod;
int use() {
Base* base = new Base();
return 43;
}

// CHECK-NOT: @_ZTSW3Mod4Base = constant
// CHECK-NOT: @_ZTIW3Mod4Base = constant
// CHECK: @_ZTVW3Mod4Base = external unnamed_addr

// CHECK-INLINE: @_ZTVW3Mod4Base = linkonce_odr {{.*}}unnamed_addr constant
// CHECK-INLINE: @_ZTSW3Mod4Base = linkonce_odr {{.*}}constant
// CHECK-INLINE: @_ZTIW3Mod4Base = linkonce_odr {{.*}}constant

// Check the case that the declaration of the key function comes from another
// module unit but the definition of the key function comes from the current
// mdoule unit.

//--- M-A.cppm
export module M:A;
export class C {
public:
virtual ~C();
};

int a_use() {
C c;
return 43;
}

//--- M-B.cppm
export module M:B;
import :A;

C::~C() {}

int b_use() {
C c;
return 43;
}

// CHECK: @_ZTVW1M1C = unnamed_addr constant
// CHECK: @_ZTSW1M1C = constant
// CHECK: @_ZTIW1M1C = constant

0 comments on commit 2d8044e

Please sign in to comment.