Skip to content

Commit

Permalink
[C++20] [Modules] Don't import function bodies from other module unit…
Browse files Browse the repository at this point in the history
…s even with optimizations

Close llvm#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.
  • Loading branch information
ChuanqiXu9 committed Nov 7, 2023
1 parent a737a33 commit 82173f3
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 9 deletions.
9 changes: 9 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3851,10 +3851,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.
if (const Module *M = F->getOwningModule();
M && M->isModulePurview() &&
getContext().getCurrentNamedModule() != M->getTopLevelModule() &&
!F->hasAttr<AlwaysInlineAttr>())
return false;

if (F->hasAttr<NoInlineAttr>())
return false;

Expand Down
10 changes: 5 additions & 5 deletions clang/test/CodeGenCXX/module-funcs-from-imports.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions clang/test/CodeGenCXX/partitions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,13 @@ 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
// CHECK: declare{{.*}} i32 @_ZW3mod3barv

// 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
72 changes: 72 additions & 0 deletions clang/test/Modules/cxx20-importing-function-bodies.cppm
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions clang/test/Modules/no-import-func-body.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit 82173f3

Please sign in to comment.