[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 28 00:54:22 PDT 2023
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/67638
>From 51c53a1aaed8fef470e699513a0f44187fbb623d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 28 Sep 2023 15:32:31 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate call to an imported module
that don't init anything
Close https://github.com/llvm/llvm-project/issues/56794
And see https://github.com/llvm/llvm-project/issues/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.
---
clang/include/clang/Basic/Module.h | 6 +++
clang/lib/Basic/Module.cpp | 2 +-
clang/lib/CodeGen/CGDeclCXX.cpp | 7 ++-
clang/lib/Sema/Sema.cpp | 21 ++++++++
clang/lib/Serialization/ASTReader.cpp | 2 +
clang/lib/Serialization/ASTWriter.cpp | 4 +-
.../module-initializer-guard-elision.cpp | 53 ++++++++++++++-----
7 files changed, 79 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index edbd2dfc2973e92..b6303cb54b9434f 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -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 NamedModuleNoInit : 1;
+
/// Describes the visibility of the various names within a
/// particular module.
enum NameVisibilityKind {
@@ -596,6 +600,8 @@ class alignas(8) Module {
return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface;
}
+ bool isNamedModuleInterfaceNoInit() const { return NamedModuleNoInit; }
+
/// Get the primary module interface name from a partition.
StringRef getPrimaryModuleInterfaceName() const {
// Technically, global module fragment belongs to global module. And global
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 4e6fbfc754b9173..82769f95bca90aa 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -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) {
+ NamedModuleNoInit(false), NameVisibility(Hidden) {
if (Parent) {
IsAvailable = Parent->isAvailable();
IsUnimportable = Parent->isUnimportable();
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 80543ba6311de45..a163069adf7c62c 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -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->isNamedModuleInterfaceNoInit())
+ continue;
llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, false);
SmallString<256> FnName;
{
@@ -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,
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index a401017d4c1c0b8..faecaa755f1bd8c 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -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->isNamedModuleInterfaceNoInit())
+ return false;
+ for (Module *I : M->Imports)
+ if (!I->isNamedModuleInterfaceNoInit())
+ return false;
+
+ return true;
+ };
+
+ CurrentModule->NamedModuleNoInit = DoesModNeedInit(CurrentModule);
+ for (Module *SubModules : CurrentModule->submodules())
+ CurrentModule->NamedModuleNoInit &= DoesModNeedInit(SubModules);
+ }
+
// Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
// modules when they are built, not every time they are used.
emitAndClearUnusedLocalTypedefWarnings();
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 0952244d037a77c..ccc201e57a6277b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5685,6 +5685,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
bool InferExportWildcard = Record[Idx++];
bool ConfigMacrosExhaustive = Record[Idx++];
bool ModuleMapIsPrivate = Record[Idx++];
+ bool NamedModuleNoInit = Record[Idx++];
Module *ParentModule = nullptr;
if (Parent)
@@ -5735,6 +5736,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
CurrentModule->InferExportWildcard = InferExportWildcard;
CurrentModule->ConfigMacrosExhaustive = ConfigMacrosExhaustive;
CurrentModule->ModuleMapIsPrivate = ModuleMapIsPrivate;
+ CurrentModule->NamedModuleNoInit = NamedModuleNoInit;
if (DeserializationListener)
DeserializationListener->ModuleRead(GlobalID, CurrentModule);
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 216ca94111e156b..b7a8807fcb04ccf 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -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)); // NamedModuleNoInit
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
unsigned DefinitionAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
@@ -2888,7 +2889,8 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
Mod->InferExplicitSubmodules,
Mod->InferExportWildcard,
Mod->ConfigMacrosExhaustive,
- Mod->ModuleMapIsPrivate};
+ Mod->ModuleMapIsPrivate,
+ Mod->NamedModuleNoInit};
Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name);
}
diff --git a/clang/test/CodeGenCXX/module-initializer-guard-elision.cpp b/clang/test/CodeGenCXX/module-initializer-guard-elision.cpp
index 2b01a670fb72764..58189ca949d8e01 100644
--- a/clang/test/CodeGenCXX/module-initializer-guard-elision.cpp
+++ b/clang/test/CodeGenCXX/module-initializer-guard-elision.cpp
@@ -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.
@@ -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;
@@ -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(){};
@@ -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
More information about the cfe-commits
mailing list