Skip to content

Commit 02b65f0

Browse files
committed
[C++20] [Modules] Don't generate call to an imported module that don't init anything
Close #56794 And see #67582 for a detailed backgrond for the issue. As required by the Itanium ABI, the module units have to generate the initialization function. However, the importers are allowed to elide the call to the initialization function if they are sure the initialization function doesn't do anything. This patch implemented this semantics.
1 parent 9744909 commit 02b65f0

File tree

7 files changed

+79
-16
lines changed

7 files changed

+79
-16
lines changed

clang/include/clang/Basic/Module.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,10 @@ class alignas(8) Module {
358358
/// to a regular (public) module map.
359359
unsigned ModuleMapIsPrivate : 1;
360360

361+
/// Whether this C++20 named modules doesn't need an initializer.
362+
/// This is only meaningful for C++20 modules.
363+
unsigned NamedModuleHasNoInit : 1;
364+
361365
/// Describes the visibility of the various names within a
362366
/// particular module.
363367
enum NameVisibilityKind {
@@ -596,6 +600,8 @@ class alignas(8) Module {
596600
return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface;
597601
}
598602

603+
bool isNamedModuleInterfaceHasNoInit() const { return NamedModuleHasNoInit; }
604+
599605
/// Get the primary module interface name from a partition.
600606
StringRef getPrimaryModuleInterfaceName() const {
601607
// Technically, global module fragment belongs to global module. And global

clang/lib/Basic/Module.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
4444
InferSubmodules(false), InferExplicitSubmodules(false),
4545
InferExportWildcard(false), ConfigMacrosExhaustive(false),
4646
NoUndeclaredIncludes(false), ModuleMapIsPrivate(false),
47-
NameVisibility(Hidden) {
47+
NamedModuleHasNoInit(false), NameVisibility(Hidden) {
4848
if (Parent) {
4949
IsAvailable = Parent->isAvailable();
5050
IsUnimportable = Parent->isUnimportable();

clang/lib/CodeGen/CGDeclCXX.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,10 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
675675
// No Itanium initializer in header like modules.
676676
if (M->isHeaderLikeModule())
677677
continue; // TODO: warn of mixed use of module map modules and C++20?
678+
// We're allowed to skip the initialization if we are sure it doesn't
679+
// do any thing.
680+
if (M->isNamedModuleInterfaceHasNoInit())
681+
continue;
678682
llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, false);
679683
SmallString<256> FnName;
680684
{
@@ -731,8 +735,7 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
731735
// If we have a completely empty initializer then we do not want to create
732736
// the guard variable.
733737
ConstantAddress GuardAddr = ConstantAddress::invalid();
734-
if (!AllImports.empty() || !PrioritizedCXXGlobalInits.empty() ||
735-
!CXXGlobalInits.empty()) {
738+
if (!ModuleInits.empty()) {
736739
// Create the guard var.
737740
llvm::GlobalVariable *Guard = new llvm::GlobalVariable(
738741
getModule(), Int8Ty, /*isConstant=*/false,

clang/lib/Sema/Sema.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,27 @@ void Sema::ActOnEndOfTranslationUnit() {
12451245
}
12461246
}
12471247

1248+
// Now we can decide whether the modules we're building need an initializer.
1249+
if (Module *CurrentModule = getCurrentModule();
1250+
CurrentModule && CurrentModule->isInterfaceOrPartition()) {
1251+
auto DoesModNeedInit = [this](Module *M) {
1252+
if (!getASTContext().getModuleInitializers(M).empty())
1253+
return false;
1254+
for (auto [Exported, _] : M->Exports)
1255+
if (!Exported->isNamedModuleInterfaceHasNoInit())
1256+
return false;
1257+
for (Module *I : M->Imports)
1258+
if (!I->isNamedModuleInterfaceHasNoInit())
1259+
return false;
1260+
1261+
return true;
1262+
};
1263+
1264+
CurrentModule->NamedModuleHasNoInit = DoesModNeedInit(CurrentModule);
1265+
for (Module *SubModules : CurrentModule->submodules())
1266+
CurrentModule->NamedModuleHasNoInit &= DoesModNeedInit(SubModules);
1267+
}
1268+
12481269
// Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
12491270
// modules when they are built, not every time they are used.
12501271
emitAndClearUnusedLocalTypedefWarnings();

clang/lib/Serialization/ASTReader.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5685,6 +5685,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
56855685
bool InferExportWildcard = Record[Idx++];
56865686
bool ConfigMacrosExhaustive = Record[Idx++];
56875687
bool ModuleMapIsPrivate = Record[Idx++];
5688+
bool NamedModuleHasNoInit = Record[Idx++];
56885689

56895690
Module *ParentModule = nullptr;
56905691
if (Parent)
@@ -5735,6 +5736,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
57355736
CurrentModule->InferExportWildcard = InferExportWildcard;
57365737
CurrentModule->ConfigMacrosExhaustive = ConfigMacrosExhaustive;
57375738
CurrentModule->ModuleMapIsPrivate = ModuleMapIsPrivate;
5739+
CurrentModule->NamedModuleHasNoInit = NamedModuleHasNoInit;
57385740
if (DeserializationListener)
57395741
DeserializationListener->ModuleRead(GlobalID, CurrentModule);
57405742

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2779,6 +2779,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
27792779
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExportWild...
27802780
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ConfigMacrosExh...
27812781
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ModuleMapIsPriv...
2782+
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // NamedModuleHasN...
27822783
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
27832784
unsigned DefinitionAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
27842785

@@ -2888,7 +2889,8 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
28882889
Mod->InferExplicitSubmodules,
28892890
Mod->InferExportWildcard,
28902891
Mod->ConfigMacrosExhaustive,
2891-
Mod->ModuleMapIsPrivate};
2892+
Mod->ModuleMapIsPrivate,
2893+
Mod->NamedModuleHasNoInit};
28922894
Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name);
28932895
}
28942896

clang/test/CodeGenCXX/module-initializer-guard-elision.cpp

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,24 @@
88
// RUN: -o - | FileCheck %s --check-prefix=CHECK-O
99

1010
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 P.cpp \
11-
// RUN: -emit-module-interface -fmodule-file=O.pcm -o P.pcm
11+
// RUN: -emit-module-interface -fmodule-file=O=O.pcm -o P.pcm
1212
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 P.pcm -S -emit-llvm \
1313
// RUN: -o - | FileCheck %s --check-prefix=CHECK-P
1414

1515
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 Q.cpp \
16-
// RUN: -emit-module-interface -fmodule-file=O.pcm -o Q.pcm
16+
// RUN: -emit-module-interface -o Q.pcm
1717
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 Q.pcm -S -emit-llvm \
18-
// RUN: -o - | FileCheck %s --check-prefix=CHECK-Q
18+
// RUN: -o - | FileCheck %s --check-prefix=CHECK-Q
19+
20+
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 R.cpp \
21+
// RUN: -emit-module-interface -fmodule-file=Q=Q.pcm -o R.pcm
22+
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 R.pcm -S -emit-llvm \
23+
// RUN: -o - | FileCheck %s --check-prefix=CHECK-R
24+
25+
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 S.cpp \
26+
// RUN: -emit-module-interface -fmodule-file=Q=Q.pcm -fmodule-file=R=R.pcm -o S.pcm
27+
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 S.pcm -S -emit-llvm \
28+
// RUN: -o - | FileCheck %s --check-prefix=CHECK-S
1929

2030
// Testing cases where we can elide the module initializer guard variable.
2131

@@ -31,8 +41,8 @@ export int foo ();
3141
// CHECK-O-NEXT: ret void
3242
// CHECK-O-NOT: @_ZGIW1O__in_chrg
3343

34-
// This has no global inits but imports a module, and therefore needs a guard
35-
// variable.
44+
// This has no global inits and all the imported modules don't need inits. So
45+
// guard variable is not needed.
3646
//--- P.cpp
3747

3848
export module P;
@@ -41,16 +51,14 @@ export import O;
4151
export int bar ();
4252

4353
// CHECK-P: define void @_ZGIW1P
44-
// CHECK-P-LABEL: init
45-
// CHECK-P: store i8 1, ptr @_ZGIW1P__in_chrg
46-
// CHECK-P: call void @_ZGIW1O()
47-
// CHECK-P-NOT: call void @__cxx_global_var_init
54+
// CHECK-P-LABEL: entry
55+
// CHECK-P-NEXT: ret void
56+
// CHECK-P-NOT: @_ZGIW1P__in_chrg
4857

49-
// This imports a module and has global inits, so needs a guard.
58+
// This has global inits, so needs a guard.
5059
//--- Q.cpp
5160

5261
export module Q;
53-
export import O;
5462

5563
export struct Quack {
5664
Quack(){};
@@ -64,6 +72,27 @@ export int baz ();
6472
// CHECK-Q: call {{.*}} @_ZNW1Q5QuackC1Ev
6573
// CHECK-Q: define void @_ZGIW1Q
6674
// CHECK-Q: store i8 1, ptr @_ZGIW1Q__in_chrg
67-
// CHECK-Q: call void @_ZGIW1O()
6875
// CHECK-Q: call void @__cxx_global_var_init
6976

77+
// This doesn't have a global init, but it imports a module which needs global
78+
// init, so needs a guard
79+
//--- R.cpp
80+
81+
export module R;
82+
export import Q;
83+
84+
// CHECK-R: define void @_ZGIW1R
85+
// CHECK-R: store i8 1, ptr @_ZGIW1R__in_chrg
86+
// CHECK-R: call{{.*}}@_ZGIW1Q
87+
88+
// This doesn't have a global init and the imported module doesn't have variables needs
89+
// dynamic initialization.
90+
// But the imported module contains modules initialization. So needs a guard.
91+
//--- S.cpp
92+
93+
export module S;
94+
export import R;
95+
96+
// CHECK-S: define void @_ZGIW1S
97+
// CHECK-S: store i8 1, ptr @_ZGIW1S__in_chrg
98+
// CHECK-S: call{{.*}}@_ZGIW1R

0 commit comments

Comments
 (0)