Skip to content

Commit 82173f3

Browse files
ChuanqiXu9ChuanqiXu9
committed
[C++20] [Modules] Don't import function bodies from other module units even with optimizations
Close #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.
1 parent a737a33 commit 82173f3

File tree

5 files changed

+91
-9
lines changed

5 files changed

+91
-9
lines changed

‎clang/lib/CodeGen/CodeGenModule.cpp

+9
Original file line numberDiff line numberDiff line change
@@ -3851,10 +3851,19 @@ CodeGenModule::isTriviallyRecursive(const FunctionDecl *FD) {
38513851
bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
38523852
if (getFunctionLinkage(GD) != llvm::Function::AvailableExternallyLinkage)
38533853
return true;
3854+
38543855
const auto *F = cast<FunctionDecl>(GD.getDecl());
38553856
if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr<AlwaysInlineAttr>())
38563857
return false;
38573858

3859+
// We don't import function bodies from other named module units since that
3860+
// behavior may break ABI compatibility of the current unit.
3861+
if (const Module *M = F->getOwningModule();
3862+
M && M->isModulePurview() &&
3863+
getContext().getCurrentNamedModule() != M->getTopLevelModule() &&
3864+
!F->hasAttr<AlwaysInlineAttr>())
3865+
return false;
3866+
38583867
if (F->hasAttr<NoInlineAttr>())
38593868
return false;
38603869

‎clang/test/CodeGenCXX/module-funcs-from-imports.cppm

+5-5
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ int use() {
6464
// CHECK-O0-NOT: non_exported_func_not_called
6565
// CHECK-O0-NOT: func_not_called
6666

67-
// Checks that all the potentially called function in the importees are generated in the importer's code
68-
// with available_externally attribute.
67+
// Checks that the generated code within optimizations keep the same behavior with
68+
// O0 to keep consistent ABI.
6969
// CHECK-O1: define{{.*}}_Z3usev(
70-
// CHECK-O1: define{{.*}}available_externally{{.*}}_ZW1M13exported_funcv(
70+
// CHECK-O1: declare{{.*}}_ZW1M13exported_funcv(
7171
// CHECK-O1: define{{.*}}available_externally{{.*}}_ZW1M18always_inline_funcv(
72-
// CHECK-O1: define{{.*}}available_externally{{.*}}_ZW1M17non_exported_funcv(
73-
// CHECK-O1: define{{.*}}available_externally{{.*}}_Z11func_in_gmfv(
72+
// CHECK-O1-NOT: func_in_gmf
7473
// CHECK-O1-NOT: func_in_gmf_not_called
74+
// CHECK-O1-NOT: non_exported_func
7575
// CHECK-O1-NOT: non_exported_func_not_called
7676
// CHECK-O1-NOT: func_not_called

‎clang/test/CodeGenCXX/partitions.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,13 @@ export int use() {
3939
return foo() + bar() + a + b;
4040
}
4141

42+
// FIXME: The definition of the variables shouldn't be exported too.
4243
// CHECK: @_ZW3mod1a = available_externally global
4344
// CHECK: @_ZW3mod1b = available_externally global
4445
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
4546
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
4647

4748
// CHECK-OPT: @_ZW3mod1a = available_externally global
4849
// CHECK-OPT: @_ZW3mod1b = available_externally global
49-
// CHECK-OPT: define available_externally{{.*}} i32 @_ZW3mod3foov
50-
// CHECK-OPT: define available_externally{{.*}} i32 @_ZW3mod3barv
50+
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
51+
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv

‎clang/test/Modules/cxx20-importing-function-bodies.cppm

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: cd %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
6+
// RUN: -emit-module-interface -o %t/a.pcm
7+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
8+
// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/b.pcm
9+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
10+
// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/c.pcm
11+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.pcm -S \
12+
// RUN: -emit-llvm -disable-llvm-passes -o - | FileCheck %t/c.cppm
13+
//
14+
// Be sure that we keep the same behavior as if optization not enabled.
15+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/a.cppm \
16+
// RUN: -emit-module-interface -o %t/a.pcm
17+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/b.cppm \
18+
// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/b.pcm
19+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/c.cppm \
20+
// RUN: -emit-module-interface -fprebuilt-module-path=%t -o %t/c.pcm
21+
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -O3 %t/c.pcm \
22+
// RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/c.cppm
23+
24+
//--- a.cppm
25+
export module a;
26+
export int a() {
27+
return 43;
28+
}
29+
30+
template <int C>
31+
int implicitly_inlined_template_function() {
32+
return C;
33+
}
34+
35+
inline int reachable_inlined_a() {
36+
return 45;
37+
}
38+
39+
int reachable_notinlined_a() {
40+
return 46;
41+
}
42+
43+
export inline int inlined_a() {
44+
return 44 + reachable_inlined_a() +
45+
reachable_notinlined_a() +
46+
implicitly_inlined_template_function<47>();
47+
}
48+
49+
//--- b.cppm
50+
export module b;
51+
export import a;
52+
export int b() {
53+
return 43 + a();
54+
}
55+
export inline int inlined_b() {
56+
return 44 + inlined_a() + a();;
57+
}
58+
59+
//--- c.cppm
60+
export module c;
61+
export import b;
62+
export int c() {
63+
return 43 + b() + a() + inlined_b() + inlined_a();
64+
}
65+
66+
// CHECK: declare{{.*}}@_ZW1b1bv
67+
// CHECK: declare{{.*}}@_ZW1a1av
68+
// CHECK: define{{.*}}@_ZW1b9inlined_bv
69+
// CHECK: define{{.*}}@_ZW1a9inlined_av
70+
// CHECK: define{{.*}}@_ZW1a19reachable_inlined_av
71+
// CHECK: declare{{.*}}@_ZW1a22reachable_notinlined_av
72+
// CHECK: define{{.*}}@_ZW1a36implicitly_inlined_template_functionILi47EEiv

‎clang/test/Modules/no-import-func-body.cppm

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ export int c() {
3838
return 43 + b() + a() + b_noinline() + a_noinline();
3939
}
4040

41-
// CHECK: define{{.*}}available_externally{{.*}}@_ZW1b1bv(
42-
// CHECK: define{{.*}}available_externally{{.*}}@_ZW1a1av(
41+
// CHECK: declare{{.*}}@_ZW1b1bv(
42+
// CHECK: declare{{.*}}@_ZW1a1av(
4343

4444
// CHECK: declare{{.*}}@_ZW1b10b_noinlinev()
4545
// CHECK: declare{{.*}}@_ZW1a10a_noinlinev()

0 commit comments

Comments
 (0)