[clang] [C++20] [Modules] Don't import function bodies from other module units even with optimizations (PR #71031)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 7 06:48:53 PST 2023
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/71031
>From 427c1e1960d48e8803e235b4d5beb437ea21c942 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 2 Nov 2023 15:19:58 +0800
Subject: [PATCH 1/2] [C++20] [Modules] Don't import function bodies from other
module units even with optimizations
Close https://github.com/llvm/llvm-project/issues/60996.
Previously, clang will try to import function bodies from other module
units to get more optimization oppotunities as much as possible. Then
the motivation becomes the direct cause of the above issue.
However, according to the discussion in SG15, the behavior of importing
function bodies from other module units breaks the ABI compatibility. It
is unwanted. So the original behavior of clang is incorrect. This patch
choose to not import function bodies from other module units in all
cases to follow the expectation.
Note that the desired optimized BMI idea is discarded too. Since it will
still break the ABI compatibility after we import function bodies
seperately.
The release note will be added seperately.
There is a similar issue for variable definitions. I'll try to handle
that in a different commit.
---
clang/lib/CodeGen/CodeGenModule.cpp | 9 +++
.../CodeGenCXX/module-funcs-from-imports.cppm | 10 +--
clang/test/CodeGenCXX/partitions.cpp | 5 +-
.../cxx20-importing-function-bodies.cppm | 72 +++++++++++++++++++
clang/test/Modules/no-import-func-body.cppm | 4 +-
5 files changed, 91 insertions(+), 9 deletions(-)
create mode 100644 clang/test/Modules/cxx20-importing-function-bodies.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cc81a68b15c4324..893e7643a447033 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3856,10 +3856,19 @@ CodeGenModule::isTriviallyRecursive(const FunctionDecl *FD) {
bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
if (getFunctionLinkage(GD) != llvm::Function::AvailableExternallyLinkage)
return true;
+
const auto *F = cast<FunctionDecl>(GD.getDecl());
if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr<AlwaysInlineAttr>())
return false;
+ // We don't import function bodies from other named module units since that
+ // behavior may break ABI compatibility of the current unit.
+ Module *M = F->getOwningModule();
+ if (M && M->isModulePurview() &&
+ getContext().getCurrentNamedModule() != M->getTopLevelModule() &&
+ !F->hasAttr<AlwaysInlineAttr>())
+ return false;
+
if (F->hasAttr<NoInlineAttr>())
return false;
diff --git a/clang/test/CodeGenCXX/module-funcs-from-imports.cppm b/clang/test/CodeGenCXX/module-funcs-from-imports.cppm
index 3b415bd9c457291..33cdf437110a9e8 100644
--- a/clang/test/CodeGenCXX/module-funcs-from-imports.cppm
+++ b/clang/test/CodeGenCXX/module-funcs-from-imports.cppm
@@ -64,13 +64,13 @@ int use() {
// CHECK-O0-NOT: non_exported_func_not_called
// CHECK-O0-NOT: func_not_called
-// Checks that all the potentially called function in the importees are generated in the importer's code
-// with available_externally attribute.
+// Checks that the generated code within optimizations keep the same behavior with
+// O0 to keep consistent ABI.
// CHECK-O1: define{{.*}}_Z3usev(
-// CHECK-O1: define{{.*}}available_externally{{.*}}_ZW1M13exported_funcv(
+// CHECK-O1: declare{{.*}}_ZW1M13exported_funcv(
// CHECK-O1: define{{.*}}available_externally{{.*}}_ZW1M18always_inline_funcv(
-// CHECK-O1: define{{.*}}available_externally{{.*}}_ZW1M17non_exported_funcv(
-// CHECK-O1: define{{.*}}available_externally{{.*}}_Z11func_in_gmfv(
+// CHECK-O1-NOT: func_in_gmf
// CHECK-O1-NOT: func_in_gmf_not_called
+// CHECK-O1-NOT: non_exported_func
// CHECK-O1-NOT: non_exported_func_not_called
// CHECK-O1-NOT: func_not_called
diff --git a/clang/test/CodeGenCXX/partitions.cpp b/clang/test/CodeGenCXX/partitions.cpp
index 865af5f161eff11..b58e88fafdc6b15 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -39,6 +39,7 @@ export int use() {
return foo() + bar() + a + b;
}
+// FIXME: The definition of the variables shouldn't be exported too.
// CHECK: @_ZW3mod1a = available_externally global
// CHECK: @_ZW3mod1b = available_externally global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
@@ -46,5 +47,5 @@ export int use() {
// CHECK-OPT: @_ZW3mod1a = available_externally global
// CHECK-OPT: @_ZW3mod1b = available_externally global
-// CHECK-OPT: define available_externally{{.*}} i32 @_ZW3mod3foov
-// CHECK-OPT: define available_externally{{.*}} i32 @_ZW3mod3barv
+// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
+// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/cxx20-importing-function-bodies.cppm b/clang/test/Modules/cxx20-importing-function-bodies.cppm
new file mode 100644
index 000000000000000..f1b8f9a8f0c9118
--- /dev/null
+++ b/clang/test/Modules/cxx20-importing-function-bodies.cppm
@@ -0,0 +1,72 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
+// RUN: -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
+// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
+// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/c.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.pcm -S \
+// RUN: -emit-llvm -disable-llvm-passes -o - | FileCheck %t/c.cppm
+//
+// Be sure that we keep the same behavior as if optization not enabled.
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/a.cppm \
+// RUN: -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/b.cppm \
+// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/c.cppm \
+// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/c.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/c.pcm \
+// RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/c.cppm
+
+//--- a.cppm
+export module a;
+export int a() {
+ return 43;
+}
+
+template <int C>
+int implicitly_inlined_template_function() {
+ return C;
+}
+
+inline int reachable_inlined_a() {
+ return 45;
+}
+
+int reachable_notinlined_a() {
+ return 46;
+}
+
+export inline int inlined_a() {
+ return 44 + reachable_inlined_a() +
+ reachable_notinlined_a() +
+ implicitly_inlined_template_function<47>();
+}
+
+//--- b.cppm
+export module b;
+export import a;
+export int b() {
+ return 43 + a();
+}
+export inline int inlined_b() {
+ return 44 + inlined_a() + a();;
+}
+
+//--- c.cppm
+export module c;
+export import b;
+export int c() {
+ return 43 + b() + a() + inlined_b() + inlined_a();
+}
+
+// CHECK: declare{{.*}}@_ZW1b1bv
+// CHECK: declare{{.*}}@_ZW1a1av
+// CHECK: define{{.*}}@_ZW1b9inlined_bv
+// CHECK: define{{.*}}@_ZW1a9inlined_av
+// CHECK: define{{.*}}@_ZW1a19reachable_inlined_av
+// CHECK: declare{{.*}}@_ZW1a22reachable_notinlined_av
+// CHECK: define{{.*}}@_ZW1a36implicitly_inlined_template_functionILi47EEiv
diff --git a/clang/test/Modules/no-import-func-body.cppm b/clang/test/Modules/no-import-func-body.cppm
index 9a111dca9855c09..6c7292b1e469bdf 100644
--- a/clang/test/Modules/no-import-func-body.cppm
+++ b/clang/test/Modules/no-import-func-body.cppm
@@ -38,8 +38,8 @@ export int c() {
return 43 + b() + a() + b_noinline() + a_noinline();
}
-// CHECK: define{{.*}}available_externally{{.*}}@_ZW1b1bv(
-// CHECK: define{{.*}}available_externally{{.*}}@_ZW1a1av(
+// CHECK: declare{{.*}}@_ZW1b1bv(
+// CHECK: declare{{.*}}@_ZW1a1av(
// CHECK: declare{{.*}}@_ZW1b10b_noinlinev()
// CHECK: declare{{.*}}@_ZW1a10a_noinlinev()
>From d44d2409698c1ab51a3f508ee8f590e8b42b9c08 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Tue, 7 Nov 2023 22:48:46 +0800
Subject: [PATCH 2/2] Update clang/lib/CodeGen/CodeGenModule.cpp
Co-authored-by: Timm Baeder <tbaeder at redhat.com>
---
clang/lib/CodeGen/CodeGenModule.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 893e7643a447033..54571fc10437496 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3863,8 +3863,8 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
// We don't import function bodies from other named module units since that
// behavior may break ABI compatibility of the current unit.
- Module *M = F->getOwningModule();
- if (M && M->isModulePurview() &&
+ if (const Module *M = F->getOwningModule();
+ M && M->isModulePurview() &&
getContext().getCurrentNamedModule() != M->getTopLevelModule() &&
!F->hasAttr<AlwaysInlineAttr>())
return false;
More information about the cfe-commits
mailing list