Skip to content

Commit

Permalink
[C++20] [Modules] Don't generate call to an imported module that dont…
Browse files Browse the repository at this point in the history
… init anything (#67638)

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.
  • Loading branch information
ChuanqiXu9 committed Sep 28, 2023
1 parent e0a48c0 commit 989173c
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 16 deletions.
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ class alignas(8) Module {
/// to a regular (public) module map.
unsigned ModuleMapIsPrivate : 1;

/// Whether this C++20 named modules doesn't need an initializer.
/// This is only meaningful for C++20 modules.
unsigned NamedModuleHasNoInit : 1;

/// Describes the visibility of the various names within a
/// particular module.
enum NameVisibilityKind {
Expand Down Expand Up @@ -596,6 +600,8 @@ class alignas(8) Module {
return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface;
}

bool isNamedModuleInterfaceHasNoInit() const { return NamedModuleHasNoInit; }

/// Get the primary module interface name from a partition.
StringRef getPrimaryModuleInterfaceName() const {
// Technically, global module fragment belongs to global module. And global
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Basic/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
InferSubmodules(false), InferExplicitSubmodules(false),
InferExportWildcard(false), ConfigMacrosExhaustive(false),
NoUndeclaredIncludes(false), ModuleMapIsPrivate(false),
NameVisibility(Hidden) {
NamedModuleHasNoInit(false), NameVisibility(Hidden) {
if (Parent) {
IsAvailable = Parent->isAvailable();
IsUnimportable = Parent->isUnimportable();
Expand Down
7 changes: 5 additions & 2 deletions clang/lib/CodeGen/CGDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,10 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
// No Itanium initializer in header like modules.
if (M->isHeaderLikeModule())
continue; // TODO: warn of mixed use of module map modules and C++20?
// We're allowed to skip the initialization if we are sure it doesn't
// do any thing.
if (M->isNamedModuleInterfaceHasNoInit())
continue;
llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, false);
SmallString<256> FnName;
{
Expand Down Expand Up @@ -731,8 +735,7 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
// If we have a completely empty initializer then we do not want to create
// the guard variable.
ConstantAddress GuardAddr = ConstantAddress::invalid();
if (!AllImports.empty() || !PrioritizedCXXGlobalInits.empty() ||
!CXXGlobalInits.empty()) {
if (!ModuleInits.empty()) {
// Create the guard var.
llvm::GlobalVariable *Guard = new llvm::GlobalVariable(
getModule(), Int8Ty, /*isConstant=*/false,
Expand Down
21 changes: 21 additions & 0 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,27 @@ void Sema::ActOnEndOfTranslationUnit() {
}
}

// Now we can decide whether the modules we're building need an initializer.
if (Module *CurrentModule = getCurrentModule();
CurrentModule && CurrentModule->isInterfaceOrPartition()) {
auto DoesModNeedInit = [this](Module *M) {
if (!getASTContext().getModuleInitializers(M).empty())
return false;
for (auto [Exported, _] : M->Exports)
if (!Exported->isNamedModuleInterfaceHasNoInit())
return false;
for (Module *I : M->Imports)
if (!I->isNamedModuleInterfaceHasNoInit())
return false;

return true;
};

CurrentModule->NamedModuleHasNoInit = DoesModNeedInit(CurrentModule);
for (Module *SubModules : CurrentModule->submodules())
CurrentModule->NamedModuleHasNoInit &= DoesModNeedInit(SubModules);
}

// Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
// modules when they are built, not every time they are used.
emitAndClearUnusedLocalTypedefWarnings();
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5685,6 +5685,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
bool InferExportWildcard = Record[Idx++];
bool ConfigMacrosExhaustive = Record[Idx++];
bool ModuleMapIsPrivate = Record[Idx++];
bool NamedModuleHasNoInit = Record[Idx++];

Module *ParentModule = nullptr;
if (Parent)
Expand Down Expand Up @@ -5735,6 +5736,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
CurrentModule->InferExportWildcard = InferExportWildcard;
CurrentModule->ConfigMacrosExhaustive = ConfigMacrosExhaustive;
CurrentModule->ModuleMapIsPrivate = ModuleMapIsPrivate;
CurrentModule->NamedModuleHasNoInit = NamedModuleHasNoInit;
if (DeserializationListener)
DeserializationListener->ModuleRead(GlobalID, CurrentModule);

Expand Down
4 changes: 3 additions & 1 deletion clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2779,6 +2779,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExportWild...
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ConfigMacrosExh...
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ModuleMapIsPriv...
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // NamedModuleHasN...
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
unsigned DefinitionAbbrev = Stream.EmitAbbrev(std::move(Abbrev));

Expand Down Expand Up @@ -2888,7 +2889,8 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
Mod->InferExplicitSubmodules,
Mod->InferExportWildcard,
Mod->ConfigMacrosExhaustive,
Mod->ModuleMapIsPrivate};
Mod->ModuleMapIsPrivate,
Mod->NamedModuleHasNoInit};
Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name);
}

Expand Down
53 changes: 41 additions & 12 deletions clang/test/CodeGenCXX/module-initializer-guard-elision.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,24 @@
// RUN: -o - | FileCheck %s --check-prefix=CHECK-O

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

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

// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 R.cpp \
// RUN: -emit-module-interface -fmodule-file=Q=Q.pcm -o R.pcm
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 R.pcm -S -emit-llvm \
// RUN: -o - | FileCheck %s --check-prefix=CHECK-R

// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 S.cpp \
// RUN: -emit-module-interface -fmodule-file=Q=Q.pcm -fmodule-file=R=R.pcm -o S.pcm
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 S.pcm -S -emit-llvm \
// RUN: -o - | FileCheck %s --check-prefix=CHECK-S

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

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

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

export module P;
Expand All @@ -41,16 +51,14 @@ export import O;
export int bar ();

// CHECK-P: define void @_ZGIW1P
// CHECK-P-LABEL: init
// CHECK-P: store i8 1, ptr @_ZGIW1P__in_chrg
// CHECK-P: call void @_ZGIW1O()
// CHECK-P-NOT: call void @__cxx_global_var_init
// CHECK-P-LABEL: entry
// CHECK-P-NEXT: ret void
// CHECK-P-NOT: @_ZGIW1P__in_chrg

// This imports a module and has global inits, so needs a guard.
// This has global inits, so needs a guard.
//--- Q.cpp

export module Q;
export import O;

export struct Quack {
Quack(){};
Expand All @@ -64,6 +72,27 @@ export int baz ();
// CHECK-Q: call {{.*}} @_ZNW1Q5QuackC1Ev
// CHECK-Q: define void @_ZGIW1Q
// CHECK-Q: store i8 1, ptr @_ZGIW1Q__in_chrg
// CHECK-Q: call void @_ZGIW1O()
// CHECK-Q: call void @__cxx_global_var_init

// This doesn't have a global init, but it imports a module which needs global
// init, so needs a guard
//--- R.cpp

export module R;
export import Q;

// CHECK-R: define void @_ZGIW1R
// CHECK-R: store i8 1, ptr @_ZGIW1R__in_chrg
// CHECK-R: call{{.*}}@_ZGIW1Q

// This doesn't have a global init and the imported module doesn't have variables needs
// dynamic initialization.
// But the imported module contains modules initialization. So needs a guard.
//--- S.cpp

export module S;
export import R;

// CHECK-S: define void @_ZGIW1S
// CHECK-S: store i8 1, ptr @_ZGIW1S__in_chrg
// CHECK-S: call{{.*}}@_ZGIW1R

0 comments on commit 989173c

Please sign in to comment.