[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
@@ -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);
ChuanqiXu9 wrote:
Thanks for the suggestion. It is addressed in the NFC patch:
https://github.com/llvm/llvm-project/commit/7e8a0e4bdc84f975772cd6d38a78c285bdd8b6cf
https://github.com/llvm/llvm-project/pull/67638
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
@@ -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);
dwblaikie wrote:
This could be, maybe:
```
CurrentModule->NamedModuleHasNoInit = DoesModNeedInit(CurrentModule) &&
llvm::all_of(CurrentModule->submodules(), DoesModNeedInit);
```
(be slightly more efficient, since it'd bail out and return false as soon as
one module needed init, rather than checking later modules)
Though the `DoesModNeedInit` might need a better name, since you'd expect that
to return `true` if the module needs init, but the "no" in "noinit" is
confusing/inverting - maybe it'd be better to name that flag
"NamedModuleHasInit" - to avoid double negatives, etc?
https://github.com/llvm/llvm-project/pull/67638
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/67638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
https://github.com/iains approved this pull request. thanks, LGTM https://github.com/llvm/llvm-project/pull/67638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/67638
>From 02b65f03b80470078107194fc1ce28680b804dab Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
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..cd86927d214fc85 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 NamedModuleHasNoInit : 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 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
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 4e6fbfc754b9173..23683eae955bd62 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) {
+ NamedModuleHasNoInit(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..18d4ed4f5290300 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->isNamedModuleInterfaceHasNoInit())
+ 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..ad85a9264d57d31 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->isNamedModuleInterfaceHasNoInit())
+return false;
+for (Module *I : M->Imports)
+ if
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
ChuanqiXu9 wrote: Opinions addressed. https://github.com/llvm/llvm-project/pull/67638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
https://github.com/iains commented: Thanks for working on this. Generally, this LGTM, but perhaps we can choose names that are unambiguously related to state - the current one could be read like an action. maybe something like: `NamedModuleHasNoInit` and `namedModuleHasNoInit()` (or I'm open to other suggestions) .. https://github.com/llvm/llvm-project/pull/67638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
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
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())
+
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 0e8a8c85f8765c086c573f36e60c895920381e18
2448304b847da467a2a81c33daa7fb789948edf1 -- clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/Sema/Sema.cpp clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/CodeGenCXX/module-initializer-guard-elision.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/include/clang/Basic/Module.h
b/clang/include/clang/Basic/Module.h
index fc8c39719979..b6303cb54b94 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -600,9 +600,7 @@ public:
return Kind == ModuleInterfaceUnit || Kind == ModulePartitionInterface;
}
- bool isNamedModuleInterfaceNoInit() const {
-return NamedModuleNoInit;
- }
+ bool isNamedModuleInterfaceNoInit() const { return NamedModuleNoInit; }
/// Get the primary module interface name from a partition.
StringRef getPrimaryModuleInterfaceName() const {
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index d2a7fcb0ee27..faecaa755f1b 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1246,8 +1246,9 @@ 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 (Module *CurrentModule = getCurrentModule();
+CurrentModule && CurrentModule->isInterfaceOrPartition()) {
+ auto DoesModNeedInit = [this](Module *M) {
if (!getASTContext().getModuleInitializers(M).empty())
return false;
for (auto [Exported, _] : M->Exports)
``
https://github.com/llvm/llvm-project/pull/67638
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
llvmbot wrote:
@llvm/pr-subscribers-clang-modules
Changes
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/67638.diff
7 Files Affected:
- (modified) clang/include/clang/Basic/Module.h (+8)
- (modified) clang/lib/Basic/Module.cpp (+1-1)
- (modified) clang/lib/CodeGen/CGDeclCXX.cpp (+5-2)
- (modified) clang/lib/Sema/Sema.cpp (+20)
- (modified) clang/lib/Serialization/ASTReader.cpp (+2)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+3-1)
- (modified) clang/test/CodeGenCXX/module-initializer-guard-elision.cpp
(+41-12)
``diff
diff --git a/clang/include/clang/Basic/Module.h
b/clang/include/clang/Basic/Module.h
index edbd2dfc2973e92..fc8c39719979ecf 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,10 @@ 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..d2a7fcb0ee27d89 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1245,6 +1245,26 @@ 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())
+
[clang] [C++20] [Modules] Don't generate call to an imported module that dont init anything (PR #67638)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/67638
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.
>From 2448304b847da467a2a81c33daa7fb789948edf1 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
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| 8 +++
clang/lib/Basic/Module.cpp| 2 +-
clang/lib/CodeGen/CGDeclCXX.cpp | 7 ++-
clang/lib/Sema/Sema.cpp | 20 +++
clang/lib/Serialization/ASTReader.cpp | 2 +
clang/lib/Serialization/ASTWriter.cpp | 4 +-
.../module-initializer-guard-elision.cpp | 53 ++-
7 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/Basic/Module.h
b/clang/include/clang/Basic/Module.h
index edbd2dfc2973e92..fc8c39719979ecf 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,10 @@ 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..d2a7fcb0ee27d89 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1245,6 +1245,26 @@ void Sema::ActOnEndOfTranslationUnit() {
}
}
+// Now we can decide whether the modules we're building need an
initializer.
+if (Module *CurrentModule =
