[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

Thanks for keeping the bot green. I see the failure comes from an ABI 
difference showed up in the test case. I'll try to address it and land it again 
later.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150023/new/

https://reviews.llvm.org/D150023

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted in dbdd6372b7af2f6df5f41d19d966e6bac1b30208 
 for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150023/new/

https://reviews.llvm.org/D150023

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

[PoYao Chang via Phabricator via cfe-commits]

rZhBoYao added a comment.

> Looks like this breaks check-clang on Mac: 
> http://45.33.8.238/macm1/62779/step_7.txt

Same here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150023/new/

https://reviews.llvm.org/D150023

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks check-clang on Mac: 
http://45.33.8.238/macm1/62779/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150023/new/

https://reviews.llvm.org/D150023

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150023: [ABI] [C++20] [Modules] Don't generate vtable if the class is defined in other module unit

[Chuanqi Xu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8a36b00d198: [ABI] [C++20] [Modules] Dont generate 
vtable if the class is defined in other… (authored by ChuanqiXu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150023/new/

https://reviews.llvm.org/D150023

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/test/CodeGenCXX/modules-vtable.cppm

Index: clang/test/CodeGenCXX/modules-vtable.cppm
===
--- /dev/null
+++ clang/test/CodeGenCXX/modules-vtable.cppm
@@ -0,0 +1,96 @@
+// 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
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1172,9 +1172,16 @@
   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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits