[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
https://github.com/Bigcheese approved this pull request. Nice performance improvement. https://github.com/llvm/llvm-project/pull/194968 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/194968 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/194968
>From 48df0dd820e47bb08f20a95dd106af0f3ef47936 Mon Sep 17 00:00:00 2001
From: Jan Svoboda
Date: Wed, 29 Apr 2026 15:58:48 -0700
Subject: [PATCH 1/4] [clang][modules] Deserialize submodule lazily
---
clang/include/clang/Basic/Module.h| 76 -
clang/include/clang/Lex/ModuleMap.h | 2 +-
clang/include/clang/Lex/Preprocessor.h| 2 +-
.../include/clang/Serialization/ASTBitCodes.h | 14 +-
clang/include/clang/Serialization/ASTReader.h | 46 +--
.../include/clang/Serialization/ModuleFile.h | 16 +
clang/lib/Basic/Module.cpp| 5 +-
clang/lib/Lex/ModuleMap.cpp | 9 +-
clang/lib/Lex/Preprocessor.cpp| 2 +-
clang/lib/Sema/SemaLookup.cpp | 3 +-
clang/lib/Sema/SemaModule.cpp | 4 +-
clang/lib/Serialization/ASTReader.cpp | 284 +-
clang/lib/Serialization/ASTWriter.cpp | 73 +++--
13 files changed, 312 insertions(+), 224 deletions(-)
diff --git a/clang/include/clang/Basic/Module.h
b/clang/include/clang/Basic/Module.h
index f83319db082d7..14200b50569c9 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -48,9 +48,17 @@ namespace clang {
class FileManager;
class LangOptions;
+class Module;
class ModuleMap;
class TargetInfo;
+/// Interface for on-demand deserialization of submodules stored in a PCM file.
+class ExternalSubmoduleSource {
+public:
+ virtual Module *getSubmodule(uint32_t GlobalID) = 0;
+ virtual ~ExternalSubmoduleSource() = default;
+};
+
/// Describes the name of a module.
using ModuleId = SmallVector, 2>;
@@ -222,6 +230,41 @@ struct ModuleAttributes {
NoUndeclaredIncludes(false) {}
};
+/// A reference to either a fully materialized Module object, or
+/// a yet-to-be-deserialized submodule in an AST file.
+class ModuleRef {
+ mutable Module *Existing = nullptr;
+ mutable ExternalSubmoduleSource *ExternalSource = nullptr;
+ mutable uint64_t SubmoduleID = 0;
+
+public:
+ ModuleRef() = default;
+ ModuleRef(Module *M) : Existing(M) {}
+ ModuleRef(ExternalSubmoduleSource *ExtSrc, uint64_t SubmoduleID)
+ : ExternalSource(ExtSrc), SubmoduleID(SubmoduleID) {}
+
+ Module *getExisting() const { return Existing; }
+ void setExisting(Module *E) { Existing = E; }
+
+ void setExternal(ExternalSubmoduleSource *ExtSrc, uint64_t ID) {
+ExternalSource = ExtSrc;
+SubmoduleID = ID;
+ }
+
+ operator bool() const { return Existing || (ExternalSource && SubmoduleID); }
+
+ operator Module *() const {
+if (ExternalSource) {
+ Existing = ExternalSource->getSubmodule(SubmoduleID);
+ ExternalSource = nullptr;
+ SubmoduleID = 0;
+}
+return Existing;
+ }
+
+ Module *operator->() const { return *this; }
+};
+
/// Required to construct a Module.
///
/// This tag type is only constructible by ModuleMap, guaranteeing it ownership
@@ -348,7 +391,7 @@ class alignas(8) Module {
private:
/// The submodules of this module, indexed by name.
- std::vector SubModules;
+ std::vector SubModules;
/// A mapping from the submodule name to the index into the
/// \c SubModules vector at which that submodule resides.
@@ -552,17 +595,17 @@ class alignas(8) Module {
/// The set of modules imported by this module, and on which this
/// module depends.
- llvm::SmallSetVector Imports;
+ llvm::SmallVector Imports;
/// The set of top-level modules that affected the compilation of this
module,
/// but were not imported.
- llvm::SmallSetVector AffectingClangModules;
+ llvm::SmallVector AffectingClangModules;
/// Describes an exported module.
///
/// The pointer is the module being re-exported, while the bit will be true
/// to indicate that this is a wildcard export.
- using ExportDecl = std::pair;
+ using ExportDecl = std::pair;
/// The set of export declarations.
SmallVector Exports;
@@ -640,7 +683,7 @@ class alignas(8) Module {
/// A conflict between two modules.
struct Conflict {
/// The module that this module conflicts with.
-Module *Other;
+ModuleRef Other;
/// The message provided to the user when there is a conflict.
std::string Message;
@@ -742,6 +785,23 @@ class alignas(8) Module {
Parent->SubModules.push_back(this);
}
+ /// Add a child submodule.
+ void addSubmodule(StringRef Name, Module *Submodule) {
+auto [It, New] = SubModuleIndex.insert({Name, SubModules.size()});
+if (New)
+ SubModules.emplace_back();
+SubModules[It->second].setExisting(Submodule);
+ }
+
+ /// Add the external part of a submodule ModuleRef.
+ void addSubmodule(StringRef Name, ExternalSubmoduleSource *ExternalSource,
+uint64_t SubmoduleID) {
+auto [It, New] = SubModuleIndex.insert({Name, SubModules.size()});
+if (New)
+ SubModules.emplace_back(
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -222,6 +230,41 @@ struct ModuleAttributes {
NoUndeclaredIncludes(false) {}
};
+/// A reference to either a fully materialized Module object, or
+/// a yet-to-be-deserialized submodule in an AST file.
+class ModuleRef {
+ mutable Module *Existing = nullptr;
+ mutable ExternalSubmoduleSource *ExternalSource = nullptr;
jansvoboda11 wrote:
You're right, I think the follow-up commit changes `operator*` to not clear
these, but that would increase traffic through the virtual function for later
calls to `operator*`. I pushed a change that uses one bit in `ExternalSource`
to signify whether it's been used to deserialize yet. This prevents calling the
virtual function repeatedly, but also keeps `ExternalSource` around for use in
the follow-up commit.
https://github.com/llvm/llvm-project/pull/194968
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -3101,6 +3098,19 @@ void ASTWriter::WriteSubmodules(Module *WritingModule,
ASTContext *Context) {
Module *Mod = Q.front();
Q.pop();
unsigned ID = getSubmoduleID(Mod);
+if (ID < FirstSubmoduleID) {
+ assert(0 && "Loaded submodule entered WritingModule ?");
jansvoboda11 wrote:
That was inspired by the logic used when writing `MacroInfosToEmit`. I think
unreachable has stronger semantics in that the branch can be optimized out in
non-assert builds, while I'd like to preserve the defensiveness of the current
scheme.
https://github.com/llvm/llvm-project/pull/194968
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -1450,7 +1450,7 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc, // Add this module to the imports list of the currently-built submodule. if (!BuildingSubmoduleStack.empty() && M != BuildingSubmoduleStack.back().M) -BuildingSubmoduleStack.back().M->Imports.insert(M); +BuildingSubmoduleStack.back().M->Imports.push_back(M); jansvoboda11 wrote: Yes. I didn't spend much time working out how to quickly check whether one `ModuleRef` that only contains `Module *` corresponds to another `ModuleRef` that only contains the `ExternalSubmoduleSource` and `SubmoduleID`. Since all tests pass, I'm assuming this would be just a performance optimization rather than correctness fix, and the performance is much improved even without the deduplication here. https://github.com/llvm/llvm-project/pull/194968 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -222,6 +230,41 @@ struct ModuleAttributes {
NoUndeclaredIncludes(false) {}
};
+/// A reference to either a fully materialized Module object, or
+/// a yet-to-be-deserialized submodule in an AST file.
+class ModuleRef {
+ mutable Module *Existing = nullptr;
+ mutable ExternalSubmoduleSource *ExternalSource = nullptr;
+ mutable uint64_t SubmoduleID = 0;
+
+public:
+ ModuleRef() = default;
+ ModuleRef(Module *M) : Existing(M) {}
+ ModuleRef(ExternalSubmoduleSource *ExtSrc, uint64_t SubmoduleID)
+ : ExternalSource(ExtSrc), SubmoduleID(SubmoduleID) {}
+
+ Module *getExisting() const { return Existing; }
+ void setExisting(Module *E) { Existing = E; }
jansvoboda11 wrote:
No, see https://github.com/llvm/llvm-project/pull/194968#discussion_r3197432750.
https://github.com/llvm/llvm-project/pull/194968
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -6277,11 +6292,34 @@ bool ASTReader::isAcceptableASTFile(
/*ValidateDiagnosticOptions=*/true);
}
-llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
- unsigned ClientLoadCapabilities) {
- // Enter the submodule block.
- if (llvm::Error Err = F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID))
-return Err;
+Module *ASTReader::getSubmodule(uint32_t GlobalID) {
+ if (GlobalID < NUM_PREDEF_SUBMODULE_IDS) {
+assert(GlobalID == 0 && "Unhandled global submodule ID");
+return nullptr;
+ }
+
+ if (GlobalID > SubmodulesLoaded.size()) {
+Error("submodule ID out of range in AST file");
+return nullptr;
+ }
+
+ SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS;
+ if (GlobalIndex < SubmodulesLoaded.size() && SubmodulesLoaded[GlobalIndex])
+return SubmodulesLoaded[GlobalIndex];
+
+ GlobalSubmoduleMapType::iterator It = GlobalSubmoduleMap.find(GlobalID);
+ assert(It != GlobalSubmoduleMap.end());
+ ModuleFile &F = *It->second;
+ unsigned Index = GlobalID - F.BaseSubmoduleID - NUM_PREDEF_SELECTOR_IDS;
+ unsigned LocalID = Index + F.LocalBaseSubmoduleID + NUM_PREDEF_SUBMODULE_IDS;
jansvoboda11 wrote:
You're right, line 6313 incorrectly uses `NUM_PREDEF_SELECTOR_IDS`. I'll fix
that.
https://github.com/llvm/llvm-project/pull/194968
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -222,6 +230,41 @@ struct ModuleAttributes {
NoUndeclaredIncludes(false) {}
};
+/// A reference to either a fully materialized Module object, or
+/// a yet-to-be-deserialized submodule in an AST file.
+class ModuleRef {
+ mutable Module *Existing = nullptr;
+ mutable ExternalSubmoduleSource *ExternalSource = nullptr;
jansvoboda11 wrote:
I don't this so, because I want `ModuleRef` to represent either the
materialized `Module *`, or the external submodule in `ASTReader`, or _both_.
This will become useful in a later patch, where even if a `Module *` gets
deserialized/materialized from an AST file, the `ExternalSubmoduleSource` is
still able to give me some information for `SubmoduleID` more efficiently than
the `Module *` object can. (More specifically - get me the set of _exported_
`ModuleRef`s without fully deserializing imports, submodules, etc. that `Module
*` would trigger.)
https://github.com/llvm/llvm-project/pull/194968
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
@@ -48,9 +48,17 @@ namespace clang {
class FileManager;
class LangOptions;
+class Module;
class ModuleMap;
class TargetInfo;
+/// Interface for on-demand deserialization of submodules stored in a PCM file.
+class ExternalSubmoduleSource {
+public:
+ virtual Module *getSubmodule(uint32_t GlobalID) = 0;
jansvoboda11 wrote:
I agree that's marginally better name, but there's already
`ASTReader::getSubmodule()` that I wanted to reuse. That avoids the need to
either update existing call sites to from `getSubmodule()` to `readSubmodule()`
(or have `ASTReader::getSubmodule()` just come a shim that forwards to
`ExternalSubmoduleSource::readSubmodule()`).
https://github.com/llvm/llvm-project/pull/194968
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/194968
>From 48df0dd820e47bb08f20a95dd106af0f3ef47936 Mon Sep 17 00:00:00 2001
From: Jan Svoboda
Date: Wed, 29 Apr 2026 15:58:48 -0700
Subject: [PATCH] [clang][modules] Deserialize submodule lazily
---
clang/include/clang/Basic/Module.h| 76 -
clang/include/clang/Lex/ModuleMap.h | 2 +-
clang/include/clang/Lex/Preprocessor.h| 2 +-
.../include/clang/Serialization/ASTBitCodes.h | 14 +-
clang/include/clang/Serialization/ASTReader.h | 46 +--
.../include/clang/Serialization/ModuleFile.h | 16 +
clang/lib/Basic/Module.cpp| 5 +-
clang/lib/Lex/ModuleMap.cpp | 9 +-
clang/lib/Lex/Preprocessor.cpp| 2 +-
clang/lib/Sema/SemaLookup.cpp | 3 +-
clang/lib/Sema/SemaModule.cpp | 4 +-
clang/lib/Serialization/ASTReader.cpp | 284 +-
clang/lib/Serialization/ASTWriter.cpp | 73 +++--
13 files changed, 312 insertions(+), 224 deletions(-)
diff --git a/clang/include/clang/Basic/Module.h
b/clang/include/clang/Basic/Module.h
index f83319db082d7..14200b50569c9 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -48,9 +48,17 @@ namespace clang {
class FileManager;
class LangOptions;
+class Module;
class ModuleMap;
class TargetInfo;
+/// Interface for on-demand deserialization of submodules stored in a PCM file.
+class ExternalSubmoduleSource {
+public:
+ virtual Module *getSubmodule(uint32_t GlobalID) = 0;
+ virtual ~ExternalSubmoduleSource() = default;
+};
+
/// Describes the name of a module.
using ModuleId = SmallVector, 2>;
@@ -222,6 +230,41 @@ struct ModuleAttributes {
NoUndeclaredIncludes(false) {}
};
+/// A reference to either a fully materialized Module object, or
+/// a yet-to-be-deserialized submodule in an AST file.
+class ModuleRef {
+ mutable Module *Existing = nullptr;
+ mutable ExternalSubmoduleSource *ExternalSource = nullptr;
+ mutable uint64_t SubmoduleID = 0;
+
+public:
+ ModuleRef() = default;
+ ModuleRef(Module *M) : Existing(M) {}
+ ModuleRef(ExternalSubmoduleSource *ExtSrc, uint64_t SubmoduleID)
+ : ExternalSource(ExtSrc), SubmoduleID(SubmoduleID) {}
+
+ Module *getExisting() const { return Existing; }
+ void setExisting(Module *E) { Existing = E; }
+
+ void setExternal(ExternalSubmoduleSource *ExtSrc, uint64_t ID) {
+ExternalSource = ExtSrc;
+SubmoduleID = ID;
+ }
+
+ operator bool() const { return Existing || (ExternalSource && SubmoduleID); }
+
+ operator Module *() const {
+if (ExternalSource) {
+ Existing = ExternalSource->getSubmodule(SubmoduleID);
+ ExternalSource = nullptr;
+ SubmoduleID = 0;
+}
+return Existing;
+ }
+
+ Module *operator->() const { return *this; }
+};
+
/// Required to construct a Module.
///
/// This tag type is only constructible by ModuleMap, guaranteeing it ownership
@@ -348,7 +391,7 @@ class alignas(8) Module {
private:
/// The submodules of this module, indexed by name.
- std::vector SubModules;
+ std::vector SubModules;
/// A mapping from the submodule name to the index into the
/// \c SubModules vector at which that submodule resides.
@@ -552,17 +595,17 @@ class alignas(8) Module {
/// The set of modules imported by this module, and on which this
/// module depends.
- llvm::SmallSetVector Imports;
+ llvm::SmallVector Imports;
/// The set of top-level modules that affected the compilation of this
module,
/// but were not imported.
- llvm::SmallSetVector AffectingClangModules;
+ llvm::SmallVector AffectingClangModules;
/// Describes an exported module.
///
/// The pointer is the module being re-exported, while the bit will be true
/// to indicate that this is a wildcard export.
- using ExportDecl = std::pair;
+ using ExportDecl = std::pair;
/// The set of export declarations.
SmallVector Exports;
@@ -640,7 +683,7 @@ class alignas(8) Module {
/// A conflict between two modules.
struct Conflict {
/// The module that this module conflicts with.
-Module *Other;
+ModuleRef Other;
/// The message provided to the user when there is a conflict.
std::string Message;
@@ -742,6 +785,23 @@ class alignas(8) Module {
Parent->SubModules.push_back(this);
}
+ /// Add a child submodule.
+ void addSubmodule(StringRef Name, Module *Submodule) {
+auto [It, New] = SubModuleIndex.insert({Name, SubModules.size()});
+if (New)
+ SubModules.emplace_back();
+SubModules[It->second].setExisting(Submodule);
+ }
+
+ /// Add the external part of a submodule ModuleRef.
+ void addSubmodule(StringRef Name, ExternalSubmoduleSource *ExternalSource,
+uint64_t SubmoduleID) {
+auto [It, New] = SubModuleIndex.insert({Name, SubModules.size()});
+if (New)
+ SubModules.emplace_back();
+
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/194968 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Deserialize submodules lazily (PR #194968)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/194968 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
