[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/76930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
ChuanqiXu9 wrote: I took another review on this patch and it shows the patch itself is not so correct. The idea of the patch is to skip GMF at first and start to write the module from the module purview directly. Then everything unused in GMF is not used. This seems fine. However, in the module purview, if we used something from a namespace of the GMF, we would everything in the namespace back. This is terrible. So in this patch, when we visit decl context, we skipped adding unreached declarations. However, this is the problem. Since an unreached declaration during the process of writing become be reachable in the end of the writing. However, we've skipped it. To solve this, either we need to write the declaration context lazily or perform a walk ahead of time. But clearly, we can't continue with the current patch. I'll bring a new one after I figured it out. https://github.com/llvm/llvm-project/pull/76930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
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 7a3b0cbb143d02b70b2bfae5cd40e9867c124748
b9a03912276d25ff819a755bef4ee72d64ce1480 --
clang/test/CXX/module/module.glob.frag/cxx20-10-4-ex2.cppm
clang/test/Modules/abi-tag.cppm clang/include/clang/AST/DeclBase.h
clang/include/clang/Serialization/ASTWriter.h clang/lib/Sema/SemaModule.cpp
clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
clang/test/CXX/module/module.import/p2.cpp
clang/test/CodeGenCXX/module-intializer-pmf.cpp
clang/test/CodeGenCXX/module-intializer.cpp clang/test/Modules/concept.cppm
clang/test/Modules/explicitly-specialized-template.cpp
clang/test/Modules/inconsistent-deduction-guide-linkage.cppm
clang/test/Modules/named-modules-adl-2.cppm
clang/test/Modules/named-modules-adl.cppm
clang/test/Modules/polluted-operator.cppm clang/test/Modules/pr58716.cppm
clang/test/Modules/pr60775.cppm clang/test/Modules/pr62589.cppm
clang/test/Modules/preferred_name.cppm
clang/test/Modules/redundant-template-default-arg3.cpp
clang/test/Modules/template-function-specialization.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 4823cbd5dd..b981a8e5bd 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3992,9 +3992,9 @@ ASTWriter::GenerateNameLookupTable(const DeclContext
*ConstDC,
if (IsDeclModuleDiscardable(D))
MarkDeclReachable(D);
} else if (llvm::all_of(Result.getLookupResult(), [this](NamedDecl *D) {
-return IsDeclModuleDiscardable(D);
- }))
-continue;
+ return IsDeclModuleDiscardable(D);
+ }))
+ continue;
// We also skip empty results. If any of the results could be external and
// the currently available results are empty, then all of the results are
@@ -5744,13 +5744,12 @@ bool ASTWriter::IsSpecialDeclNotDiscardable(Decl *D) {
//
// This may imply that the ODR checking process is context sensitive.
// That said, the same type in different module units can be considered to be
- // different if some module units discard the unused `__gnu_cxx::__cxx11`
namespace
- // while other module units don't. This is incorrect.
+ // different if some module units discard the unused `__gnu_cxx::__cxx11`
+ // namespace while other module units don't. This is incorrect.
//
- // This is a workaround to make things happen but we indeed to fix the ODR
checking
- // process indeed.
- if (auto *ND = dyn_cast(D);
- ND && ND->getAttr()) {
+ // This is a workaround to make things happen but we indeed to fix the ODR
+ // checking process indeed.
+ if (auto *ND = dyn_cast(D); ND && ND->getAttr()) {
MarkDeclReachable(D);
return true;
}
@@ -5779,7 +5778,8 @@ bool ASTWriter::IsDeclModuleDiscardable(const Decl
*ConstD) {
DC = DC->getParent()->getNonTransparentContext();
assert(DC && "Why is the decl not covered by file context?");
- if (!DC->isFileContext() &&
!cast(DC)->isDiscardedInGlobalModuleFragment()) {
+ if (!DC->isFileContext() &&
+ !cast(DC)->isDiscardedInGlobalModuleFragment()) {
MarkDeclReachable(D);
return false;
}
@@ -5835,7 +5835,8 @@ DeclID ASTWriter::GetDeclRef(const Decl *D) {
if (D->isFromASTFile())
return D->getGlobalID();
- assert(!D->isDiscardedInGlobalModuleFragment() && "We shouldn't write
discarded decl.\n");
+ assert(!D->isDiscardedInGlobalModuleFragment() &&
+ "We shouldn't write discarded decl.\n");
assert(!(reinterpret_cast(D) & 0x01) && "Invalid decl pointer");
DeclID = DeclIDs[D];
``
https://github.com/llvm/llvm-project/pull/76930
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
llvmbot wrote:
@llvm/pr-subscribers-clang-modules
@llvm/pr-subscribers-clang
Author: Chuanqi Xu (ChuanqiXu9)
Changes
This was a patch to try to implement eliding unreachable decls in GMF in
ASTWriter. It was developed a half year ago and I just rebased it but I did't
fix the failing test. It ran well.
The core idea of the patch is that we can implement the idea **reachable** in
ASTWriter naturally.
The secret is that we skip writing GMF initially (generally we will write decls
from the top to the bottom) and we start to write the declarations from module
purview. Then we will only write the declarations in GMF if it is mentioned
during the writing process. So the unreachable decls won't be written natually.
The experience in implementing this patch is pretty smooth and the tests from
the spec can be passed. I felt this should be the natural way to implement this
feature.
The only one and big problem is that we didn't implement the formal semantics
in the spec in this way : |
---
Patch is 32.57 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/76930.diff
26 Files Affected:
- (modified) clang/include/clang/AST/DeclBase.h (+10-6)
- (modified) clang/include/clang/Basic/LangOptions.def (+1)
- (modified) clang/include/clang/Driver/Options.td (+7)
- (modified) clang/include/clang/Serialization/ASTWriter.h (+24)
- (modified) clang/lib/Sema/SemaModule.cpp (+5-1)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+3)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+151-9)
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+2)
- (modified) clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
(+2-4)
- (added) clang/test/CXX/module/module.glob.frag/cxx20-10-4-ex2.cppm (+72)
- (modified) clang/test/CXX/module/module.import/p2.cpp (+4-3)
- (modified) clang/test/CodeGenCXX/module-intializer-pmf.cpp (+4-2)
- (modified) clang/test/CodeGenCXX/module-intializer.cpp (+18-21)
- (added) clang/test/Modules/abi-tag.cppm (+69)
- (modified) clang/test/Modules/concept.cppm (+2)
- (modified) clang/test/Modules/explicitly-specialized-template.cpp (+1-1)
- (modified) clang/test/Modules/inconsistent-deduction-guide-linkage.cppm (+6)
- (modified) clang/test/Modules/named-modules-adl-2.cppm (+2)
- (modified) clang/test/Modules/named-modules-adl.cppm (+2)
- (modified) clang/test/Modules/polluted-operator.cppm (+9-2)
- (modified) clang/test/Modules/pr58716.cppm (+2)
- (modified) clang/test/Modules/pr60775.cppm (+4)
- (modified) clang/test/Modules/pr62589.cppm (+3)
- (modified) clang/test/Modules/preferred_name.cppm (+2)
- (modified) clang/test/Modules/redundant-template-default-arg3.cpp (+9)
- (modified) clang/test/Modules/template-function-specialization.cpp (+2-4)
``diff
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 10dcbdb262d84e..523b930d59645a 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -233,9 +233,12 @@ class alignas(8) Decl {
/// This declaration has an owning module, but is only visible to
/// lookups that occur within that module.
-/// The discarded declarations in global module fragment belongs
-/// to this group too.
-ModulePrivate
+ModulePrivate,
+
+/// This declaration is part of a Global Module Fragment, it is permitted
+/// to discard it and therefore it is not reachable or visible to importers
+/// of the named module of which the GMF is part.
+ModuleDiscardable
};
protected:
@@ -658,9 +661,10 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;
- /// FIXME: Implement discarding declarations actually in global module
- /// fragment. See [module.global.frag]p3,4 for details.
- bool isDiscardedInGlobalModuleFragment() const { return false; }
+ /// See [module.global.frag]p3,4 for details.
+ bool isDiscardedInGlobalModuleFragment() const {
+return getModuleOwnershipKind() == ModuleOwnershipKind::ModuleDiscardable;
+ }
/// Return true if this declaration has an attribute which acts as
/// definition of the entity, such as 'alias' or 'ifunc'.
diff --git a/clang/include/clang/Basic/LangOptions.def
b/clang/include/clang/Basic/LangOptions.def
index 21abc346cf17ac..97ba8147bdfbb5 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -178,6 +178,7 @@ LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin
headers belong to system m
BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
"compiling a module interface")
BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
+LANGOPT(DiscardGMFDecls , 1, 1, "Discard unreachable decls in GMF")
BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a
corresponding object file")
BENIGN_LANGOPT(CacheGeneratedPCH, 1,
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
https://github.com/ChuanqiXu9 converted_to_draft https://github.com/llvm/llvm-project/pull/76930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/76930
This was a patch to try to implement eliding unreachable decls in GMF in
ASTWriter. It was developed a half year ago and I just rebased it but I did't
fix the failing test. It ran well.
The core idea of the patch is that we can implement the idea **reachable** in
ASTWriter naturally.
The secret is that we skip writing GMF initially (generally we will write decls
from the top to the bottom) and we start to write the declarations from module
purview. Then we will only write the declarations in GMF if it is mentioned
during the writing process. So the unreachable decls won't be written natually.
The experience in implementing this patch is pretty smooth and the tests from
the spec can be passed. I felt this should be the natural way to implement this
feature.
The only one and big problem is that we didn't implement the formal semantics
in the spec in this way : |
>From b9a03912276d25ff819a755bef4ee72d64ce1480 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Thu, 4 Jan 2024 17:24:23 +0800
Subject: [PATCH] [C++20] [Modules] Implementing Eliding Unreachable Decls of
GMF in ASTWriter
---
clang/include/clang/AST/DeclBase.h| 16 +-
clang/include/clang/Basic/LangOptions.def | 1 +
clang/include/clang/Driver/Options.td | 7 +
clang/include/clang/Serialization/ASTWriter.h | 24 +++
clang/lib/Sema/SemaModule.cpp | 6 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 3 +
clang/lib/Serialization/ASTWriter.cpp | 160 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 2 +
.../basic.scope/basic.scope.namespace/p2.cpp | 6 +-
.../module.glob.frag/cxx20-10-4-ex2.cppm | 72
clang/test/CXX/module/module.import/p2.cpp| 7 +-
.../test/CodeGenCXX/module-intializer-pmf.cpp | 6 +-
clang/test/CodeGenCXX/module-intializer.cpp | 39 ++---
clang/test/Modules/abi-tag.cppm | 69
clang/test/Modules/concept.cppm | 2 +
.../explicitly-specialized-template.cpp | 2 +-
.../inconsistent-deduction-guide-linkage.cppm | 6 +
clang/test/Modules/named-modules-adl-2.cppm | 2 +
clang/test/Modules/named-modules-adl.cppm | 2 +
clang/test/Modules/polluted-operator.cppm | 11 +-
clang/test/Modules/pr58716.cppm | 2 +
clang/test/Modules/pr60775.cppm | 4 +
clang/test/Modules/pr62589.cppm | 3 +
clang/test/Modules/preferred_name.cppm| 2 +
.../redundant-template-default-arg3.cpp | 9 +
.../template-function-specialization.cpp | 6 +-
26 files changed, 416 insertions(+), 53 deletions(-)
create mode 100644 clang/test/CXX/module/module.glob.frag/cxx20-10-4-ex2.cppm
create mode 100644 clang/test/Modules/abi-tag.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 10dcbdb262d84e..523b930d59645a 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -233,9 +233,12 @@ class alignas(8) Decl {
/// This declaration has an owning module, but is only visible to
/// lookups that occur within that module.
-/// The discarded declarations in global module fragment belongs
-/// to this group too.
-ModulePrivate
+ModulePrivate,
+
+/// This declaration is part of a Global Module Fragment, it is permitted
+/// to discard it and therefore it is not reachable or visible to importers
+/// of the named module of which the GMF is part.
+ModuleDiscardable
};
protected:
@@ -658,9 +661,10 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;
- /// FIXME: Implement discarding declarations actually in global module
- /// fragment. See [module.global.frag]p3,4 for details.
- bool isDiscardedInGlobalModuleFragment() const { return false; }
+ /// See [module.global.frag]p3,4 for details.
+ bool isDiscardedInGlobalModuleFragment() const {
+return getModuleOwnershipKind() == ModuleOwnershipKind::ModuleDiscardable;
+ }
/// Return true if this declaration has an attribute which acts as
/// definition of the entity, such as 'alias' or 'ifunc'.
diff --git a/clang/include/clang/Basic/LangOptions.def
b/clang/include/clang/Basic/LangOptions.def
index 21abc346cf17ac..97ba8147bdfbb5 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -178,6 +178,7 @@ LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin
headers belong to system m
BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
"compiling a module interface")
BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
+LANGOPT(DiscardGMFDecls , 1, 1, "Discard unreachable decls in GMF")
BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a
corresponding
