[clang] [Serialization] No transitive change for MacroID and PreprocessedEntityID #166346
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Chuanqi Xu (ChuanqiXu9) ChangesSimilar to previous no transitive changes to decls, types, identifiers and source locations ( This patch does the same thing for MacroID and PreprocessedEntityID. Some backgroundPreviously we record different IDs linearly. That is, when writing a module, if we have 17 decls in imported modules, the ID of decls in the module will start from 18. This makes the contents of the BMI changes if the we add/remove any decls, types, identifiers and source locations in the imported modules. This makes it hard for us to reduce recompilations with modules. We want to skip recompilations as we think the modules can help us to remove fake dependencies. This can be done by split the ID into <ModuleIndex, LocalIndex> pairs. Now we want to extend this to MacroID and PreprocessedEntityID. This is helpful for us in the downstream as we allowed named modules to export macros. But I believe this is also helpful for header-like modules if you'd like to explore the area. And also I think this is a nice cleanup too. Given the use of MacroID and PreprocessedEntityID are not as complicated as other IDs in the above series, I feel the patch itself should be good. I hope the vendors can test the patch to make sure it won't affect existing users. Patch is 22.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166346.diff 7 Files Affected:
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 5d09d5536e5ab..d7d429eacd67a 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -151,14 +151,14 @@ struct UnsafeQualTypeDenseMapInfo {
};
/// An ID number that refers to a macro in an AST file.
-using MacroID = uint32_t;
+using MacroID = uint64_t;
/// A global ID number that refers to a macro in an AST file.
-using GlobalMacroID = uint32_t;
+using GlobalMacroID = uint64_t;
/// A local to a module ID number that refers to a macro in an
/// AST file.
-using LocalMacroID = uint32_t;
+using LocalMacroID = uint64_t;
/// The number of predefined macro IDs.
const unsigned int NUM_PREDEF_MACRO_IDS = 1;
@@ -179,7 +179,7 @@ using CXXCtorInitializersID = uint32_t;
/// An ID number that refers to an entity in the detailed
/// preprocessing record.
-using PreprocessedEntityID = uint32_t;
+using PreprocessedEntityID = uint64_t;
/// An ID number that refers to a submodule in a module file.
using SubmoduleID = uint32_t;
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index af856a8097ab1..1cee91c48c927 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -800,14 +800,6 @@ class ASTReader
/// files.
llvm::DenseSet<LoadedMacroInfo> LoadedUndefs;
- using GlobalMacroMapType =
- ContinuousRangeMap<serialization::MacroID, ModuleFile *, 4>;
-
- /// Mapping from global macro IDs to the module in which the
- /// macro resides along with the offset that should be added to the
- /// global macro ID to produce a local ID.
- GlobalMacroMapType GlobalMacroMap;
-
/// A vector containing submodules that have already been loaded.
///
/// This vector is indexed by the Submodule ID (-1). NULL submodule entries
@@ -1655,7 +1647,7 @@ class ASTReader
/// Returns the first preprocessed entity ID that begins or ends after
/// \arg Loc.
- serialization::PreprocessedEntityID
+ unsigned
findPreprocessedEntity(SourceLocation Loc, bool EndsAfter) const;
/// Find the next module that contains entities and return the ID
@@ -1664,7 +1656,7 @@ class ASTReader
/// \param SLocMapI points at a chunk of a module that contains no
/// preprocessed entities or the entities it contains are not the
/// ones we are looking for.
- serialization::PreprocessedEntityID
+ unsigned
findNextPreprocessedEntity(
GlobalSLocOffsetMapType::const_iterator SLocMapI) const;
@@ -1748,6 +1740,13 @@ class ASTReader
std::pair<ModuleFile *, unsigned>
translateIdentifierIDToIndex(serialization::IdentifierID ID) const;
+ /// Translate an \param MacroID ID to the index of MacrosLoaded
+ /// array and the corresponding module file.
+ std::pair<ModuleFile *, unsigned>
+ translateMacroIDToIndex(serialization::MacroID ID) const;
+
+ unsigned translatePreprocessedEntityIDToIndex(serialization::PreprocessedEntityID ID) const;
+
/// Translate an \param TypeID ID to the index of TypesLoaded
/// array and the corresponding module file.
std::pair<ModuleFile *, unsigned>
@@ -2388,7 +2387,7 @@ class ASTReader
/// Retrieve the global macro ID corresponding to the given local
/// ID within the given module file.
- serialization::MacroID getGlobalMacroID(ModuleFile &M, unsigned LocalID);
+ serialization::MacroID getGlobalMacroID(ModuleFile &M, serialization::MacroID LocalID);
/// Read the source location entry with index ID.
bool ReadSLocEntry(int ID) override;
@@ -2573,7 +2572,7 @@ class ASTReader
/// Determine the global preprocessed entity ID that corresponds to
/// the given local ID within the given module.
serialization::PreprocessedEntityID
- getGlobalPreprocessedEntityID(ModuleFile &M, unsigned LocalID) const;
+ getGlobalPreprocessedEntityID(ModuleFile &M, serialization::PreprocessedEntityID LocalID) const;
/// Add a macro to deserialize its macro directive history.
///
diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h
index f20cb2f9f35ae..783e2ba7a1f94 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -353,9 +353,6 @@ class ModuleFile {
/// Base macro ID for macros local to this module.
serialization::MacroID BaseMacroID = 0;
- /// Remapping table for macro IDs in this module.
- ContinuousRangeMap<uint32_t, int, 2> MacroRemap;
-
/// The offset of the start of the set of defined macros.
uint64_t MacroStartOffset = 0;
@@ -372,9 +369,6 @@ class ModuleFile {
/// this module.
serialization::PreprocessedEntityID BasePreprocessedEntityID = 0;
- /// Remapping table for preprocessed entity IDs in this module.
- ContinuousRangeMap<uint32_t, int, 2> PreprocessedEntityRemap;
-
const PPEntityOffset *PreprocessedEntityOffsets = nullptr;
unsigned NumPreprocessedEntities = 0;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index e3106f8d8e13c..8681ae7bb6c36 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2228,9 +2228,10 @@ MacroInfo *ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset) {
// We have a macro definition. Register the association
PreprocessedEntityID
GlobalID = getGlobalPreprocessedEntityID(F, Record[NextIndex]);
+ unsigned Index = translatePreprocessedEntityIDToIndex(GlobalID);
PreprocessingRecord &PPRec = *PP.getPreprocessingRecord();
PreprocessingRecord::PPEntityID PPID =
- PPRec.getPPEntityID(GlobalID - 1, /*isLoaded=*/true);
+ PPRec.getPPEntityID(Index, /*isLoaded=*/true);
MacroDefinitionRecord *PPDef = cast_or_null<MacroDefinitionRecord>(
PPRec.getPreprocessedEntity(PPID));
if (PPDef)
@@ -2261,16 +2262,22 @@ MacroInfo *ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset) {
PreprocessedEntityID
ASTReader::getGlobalPreprocessedEntityID(ModuleFile &M,
- unsigned LocalID) const {
+ PreprocessedEntityID LocalID) const {
if (!M.ModuleOffsetMap.empty())
ReadModuleOffsetMap(M);
- ContinuousRangeMap<uint32_t, int, 2>::const_iterator
- I = M.PreprocessedEntityRemap.find(LocalID - NUM_PREDEF_PP_ENTITY_IDS);
- assert(I != M.PreprocessedEntityRemap.end()
- && "Invalid index into preprocessed entity index remap");
+ unsigned ModuleFileIndex = LocalID >> 32;
+ LocalID &= llvm::maskTrailingOnes<PreprocessedEntityID>(32);
+ ModuleFile *MF =
+ ModuleFileIndex ? M.TransitiveImports[ModuleFileIndex - 1] : &M;
+ assert(MF && "malformed identifier ID encoding?");
- return LocalID + I->second;
+ if (!ModuleFileIndex) {
+ assert(LocalID >= NUM_PREDEF_PP_ENTITY_IDS);
+ LocalID -= NUM_PREDEF_PP_ENTITY_IDS;
+ }
+
+ return ((PreprocessedEntityID)(MF->Index + 1) << 32) | LocalID;
}
OptionalFileEntryRef
@@ -4107,8 +4114,6 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
assert(Blob.size() % sizeof(PPEntityOffset) == 0);
F.NumPreprocessedEntities = Blob.size() / sizeof(PPEntityOffset);
- unsigned LocalBasePreprocessedEntityID = Record[0];
-
unsigned StartingID;
if (!PP.getPreprocessingRecord())
PP.createPreprocessingRecord();
@@ -4123,12 +4128,6 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
// Introduce the global -> local mapping for preprocessed entities in
// this module.
GlobalPreprocessedEntityMap.insert(std::make_pair(StartingID, &F));
-
- // Introduce the local -> global mapping for preprocessed entities in
- // this module.
- F.PreprocessedEntityRemap.insertOrReplace(
- std::make_pair(LocalBasePreprocessedEntityID,
- F.BasePreprocessedEntityID - LocalBasePreprocessedEntityID));
}
break;
@@ -4339,21 +4338,11 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
"duplicate MACRO_OFFSET record in AST file");
F.MacroOffsets = (const uint32_t *)Blob.data();
F.LocalNumMacros = Record[0];
- unsigned LocalBaseMacroID = Record[1];
- F.MacroOffsetsBase = Record[2] + F.ASTBlockStartOffset;
+ F.MacroOffsetsBase = Record[1] + F.ASTBlockStartOffset;
F.BaseMacroID = getTotalNumMacros();
- if (F.LocalNumMacros > 0) {
- // Introduce the global -> local mapping for macros within this module.
- GlobalMacroMap.insert(std::make_pair(getTotalNumMacros() + 1, &F));
-
- // Introduce the local -> global mapping for macros within this module.
- F.MacroRemap.insertOrReplace(
- std::make_pair(LocalBaseMacroID,
- F.BaseMacroID - LocalBaseMacroID));
-
+ if (F.LocalNumMacros > 0)
MacrosLoaded.resize(MacrosLoaded.size() + F.LocalNumMacros);
- }
break;
}
@@ -4459,8 +4448,6 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const {
F.ModuleOffsetMap = StringRef();
using RemapBuilder = ContinuousRangeMap<uint32_t, int, 2>::Builder;
- RemapBuilder MacroRemap(F.MacroRemap);
- RemapBuilder PreprocessedEntityRemap(F.PreprocessedEntityRemap);
RemapBuilder SubmoduleRemap(F.SubmoduleRemap);
RemapBuilder SelectorRemap(F.SelectorRemap);
@@ -4490,10 +4477,6 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const {
ImportedModuleVector.push_back(OM);
- uint32_t MacroIDOffset =
- endian::readNext<uint32_t, llvm::endianness::little>(Data);
- uint32_t PreprocessedEntityIDOffset =
- endian::readNext<uint32_t, llvm::endianness::little>(Data);
uint32_t SubmoduleIDOffset =
endian::readNext<uint32_t, llvm::endianness::little>(Data);
uint32_t SelectorIDOffset =
@@ -4507,9 +4490,6 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const {
static_cast<int>(BaseOffset - Offset)));
};
- mapOffset(MacroIDOffset, OM->BaseMacroID, MacroRemap);
- mapOffset(PreprocessedEntityIDOffset, OM->BasePreprocessedEntityID,
- PreprocessedEntityRemap);
mapOffset(SubmoduleIDOffset, OM->BaseSubmoduleID, SubmoduleRemap);
mapOffset(SelectorIDOffset, OM->BaseSelectorID, SelectorRemap);
}
@@ -6718,11 +6698,21 @@ SourceRange ASTReader::ReadSkippedRange(unsigned GlobalIndex) {
return Range;
}
+unsigned ASTReader::translatePreprocessedEntityIDToIndex(PreprocessedEntityID ID) const {
+ unsigned ModuleFileIndex = ID >> 32;
+ assert(ModuleFileIndex && "not translating loaded MacroID?");
+ assert(getModuleManager().size() > ModuleFileIndex - 1);
+ ModuleFile &MF = getModuleManager()[ModuleFileIndex - 1];
+
+ ID &= llvm::maskTrailingOnes<PreprocessedEntityID>(32);
+ return MF.BasePreprocessedEntityID + ID;
+}
+
PreprocessedEntity *ASTReader::ReadPreprocessedEntity(unsigned Index) {
- PreprocessedEntityID PPID = Index+1;
std::pair<ModuleFile *, unsigned> PPInfo = getModulePreprocessedEntity(Index);
ModuleFile &M = *PPInfo.first;
unsigned LocalIndex = PPInfo.second;
+ PreprocessedEntityID PPID = (((PreprocessedEntityID)M.Index + 1) << 32) | LocalIndex;
const PPEntityOffset &PPOffs = M.PreprocessedEntityOffsets[LocalIndex];
if (!PP.getPreprocessingRecord()) {
@@ -6770,8 +6760,9 @@ PreprocessedEntity *ASTReader::ReadPreprocessedEntity(unsigned Index) {
else {
PreprocessedEntityID GlobalID =
getGlobalPreprocessedEntityID(M, Record[1]);
+ unsigned Index = translatePreprocessedEntityIDToIndex(GlobalID);
Def = cast<MacroDefinitionRecord>(
- PPRec.getLoadedPreprocessedEntity(GlobalID - 1));
+ PPRec.getLoadedPreprocessedEntity(Index));
}
MacroExpansion *ME;
@@ -6824,7 +6815,7 @@ PreprocessedEntity *ASTReader::ReadPreprocessedEntity(unsigned Index) {
/// \param SLocMapI points at a chunk of a module that contains no
/// preprocessed entities or the entities it contains are not the ones we are
/// looking for.
-PreprocessedEntityID ASTReader::findNextPreprocessedEntity(
+unsigned ASTReader::findNextPreprocessedEntity(
GlobalSLocOffsetMapType::const_iterator SLocMapI) const {
++SLocMapI;
for (GlobalSLocOffsetMapType::const_iterator
@@ -6868,7 +6859,7 @@ struct PPEntityComp {
} // namespace
-PreprocessedEntityID ASTReader::findPreprocessedEntity(SourceLocation Loc,
+unsigned ASTReader::findPreprocessedEntity(SourceLocation Loc,
bool EndsAfter) const {
if (SourceMgr.isLocalSourceLocation(Loc))
return getTotalNumPreprocessedEntities();
@@ -6929,9 +6920,9 @@ std::pair<unsigned, unsigned>
return std::make_pair(0,0);
assert(!SourceMgr.isBeforeInTranslationUnit(Range.getEnd(),Range.getBegin()));
- PreprocessedEntityID BeginID =
+ unsigned BeginID =
findPreprocessedEntity(Range.getBegin(), false);
- PreprocessedEntityID EndID = findPreprocessedEntity(Range.getEnd(), true);
+ unsigned EndID = findPreprocessedEntity(Range.getEnd(), true);
return std::make_pair(BeginID, EndID);
}
@@ -8956,7 +8947,7 @@ LLVM_DUMP_METHOD void ASTReader::dump() {
llvm::errs() << "*** PCH/ModuleFile Remappings:\n";
dumpModuleIDMap("Global bit offset map", GlobalBitOffsetsMap);
dumpModuleIDMap("Global source location entry map", GlobalSLocEntryMap);
- dumpModuleIDMap("Global macro map", GlobalMacroMap);
+
dumpModuleIDMap("Global submodule map", GlobalSubmoduleMap);
dumpModuleIDMap("Global selector map", GlobalSelectorMap);
dumpModuleIDMap("Global preprocessed entity map",
@@ -9739,6 +9730,21 @@ IdentifierID ASTReader::getGlobalIdentifierID(ModuleFile &M, uint64_t LocalID) {
return ((IdentifierID)(MF->Index + 1) << 32) | LocalID;
}
+std::pair<ModuleFile *, unsigned>
+ASTReader::translateMacroIDToIndex(MacroID ID) const {
+ if (ID == 0)
+ return {nullptr, 0};
+
+ unsigned ModuleFileIndex = ID >> 32;
+ assert(ModuleFileIndex && "not translating loaded MacroID?");
+ assert(getModuleManager().size() > ModuleFileIndex - 1);
+ ModuleFile &MF = getModuleManager()[ModuleFileIndex - 1];
+
+ unsigned LocalID = ID & llvm::maskTrailingOnes<MacroID>(32);
+ assert(LocalID < MF.LocalNumMacros);
+ return {&MF, MF.BaseMacroID + LocalID};
+}
+
MacroInfo *ASTReader::getMacro(MacroID ID) {
if (ID == 0)
return nullptr;
@@ -9748,36 +9754,42 @@ MacroInfo *ASTReader::getMacro(MacroID ID) {
return nullptr;
}
- ID -= NUM_PREDEF_MACRO_IDS;
- if (!MacrosLoaded[ID]) {
- GlobalMacroMapType::iterator I
- = GlobalMacroMap.find(ID + NUM_PREDEF_MACRO_IDS);
- assert(I != GlobalMacroMap.end() && "Corrupted global macro map");
- ModuleFile *M = I->second;
- unsigned Index = ID - M->BaseMacroID;
- MacrosLoaded[ID] =
- ReadMacroRecord(*M, M->MacroOffsetsBase + M->MacroOffsets[Index]);
+ auto [M, Index] = translateMacroIDToIndex(ID);
+ if (!MacrosLoaded[Index]) {
+ assert(M != nullptr && "Untranslated Macro ID?");
+ assert(Index >= M->BaseMacroID);
+ unsigned LocalIndex = Index - M->BaseMacroID;
+ uint64_t DataOffset =
+ M->MacroOffsetsBase + M->MacroOffsets[LocalIndex];
+ MacrosLoaded[Index] = ReadMacroRecord(*M, DataOffset);
if (DeserializationListener)
- DeserializationListener->MacroRead(ID + NUM_PREDEF_MACRO_IDS,
- MacrosLoaded[ID]);
+ DeserializationListener->MacroRead(ID,
+ MacrosLoaded[Index]);
}
- return MacrosLoaded[ID];
+ return MacrosLoaded[Index];
}
-MacroID ASTReader::getGlobalMacroID(ModuleFile &M, unsigned LocalID) {
+MacroID ASTReader::getGlobalMacroID(ModuleFile &M, MacroID LocalID) {
if (LocalID < NUM_PREDEF_MACRO_IDS)
return LocalID;
if (!M.ModuleOffsetMap.empty())
ReadModuleOffsetMap(M);
- ContinuousRangeMap<uint32_t, int, 2>::iterator I
- = M.MacroRemap.find(LocalID - NUM_PREDEF_MACRO_IDS);
- assert(I != M.MacroRemap.end() && "Invalid index into macro index remap");
+ unsigned ModuleFileIndex = LocalID >> 32;
+ LocalID &= llvm::maskTrailingOnes<MacroID>(32);
+ ModuleFile *MF =
+ ModuleFileIndex ? M.TransitiveImports[ModuleFileIndex - 1] : &M;
+ assert(MF && "malformed identifier ID encoding?");
- return LocalID + I->second;
+ if (!ModuleFileIndex) {
+ assert(LocalID >= NUM_PREDEF_MACRO_IDS);
+ LocalID -= NUM_PREDEF_MACRO_IDS;
+ }
+
+ return ((MacroID)(MF->Index + 1) << 32) | LocalID;
}
serialization::SubmoduleID
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 3ac338e013deb..b5f6d040c771a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2819,14 +2819,12 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
auto Abbrev = std::make_shared<BitCodeAbbrev>();
Abbrev->Add(BitCodeAbbrevOp(MACRO_OFFSET));
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // # of macros
- Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // first ID
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32)); // base offset
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
unsigned MacroOffsetAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
{
RecordData::value_type Record[] = {MACRO_OFFSET, MacroOffsets.size(),
- FirstMacroID - NUM_PREDEF_MACRO_IDS,
MacroOffsetsBase - ASTBlockStartOffset};
Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
}
@@ -2859,9 +2857,7 @@ void ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec,
InclusionAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
}
- unsigned FirstPreprocessorEntityID
- = (Chain ? PPRec.getNumLoadedPreprocessedEntities() : 0)
- + NUM_PREDEF_PP_ENTITY_IDS;
+ unsigned FirstPreprocessorEntityID = NUM_PREDEF_PP_ENTITY_IDS;
unsigned NextPreprocessorEntityID = FirstPreprocessorEntityID;
RecordData Record;
for (PreprocessingRecord::iterator E = PPRec.local_begin(),
@@ -2925,13 +2921,10 @@ void ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec,
auto Abbrev = std::make_shared<BitCodeAbbrev>();
Abbrev->Add(BitCodeAbbrevOp(PPD_ENTITIES_OFFSETS));
- Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // first pp entity
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
unsigned PPEOffsetAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
- RecordData::value_type Record[] = {PPD_ENTITIES_OFFSETS,
- FirstPreprocessorEntityID -
- NUM_PREDEF_PP_ENTITY_IDS};
+ RecordData::value_type Record[] = {PPD_ENTITIES_OFFSETS};
Stream.EmitRecordWithBlob(PPEOffsetAbbrev, Record,
bytes(PreprocessedEntityOffsets));
}
@@ -6099,9 +6092,6 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot,
// These values should be unique within a chain, since they will be read
// as keys into ContinuousRangeMaps.
- writeBaseIDOrNone(M.BaseMacroID, M.LocalNumMacros);
- writeBaseIDOrNone(M.BasePreprocessedEntityID,
- M.NumPreprocessedEntities);
writeBaseIDOrNone(M.BaseSubmoduleID, M.LocalNumSubmodules);
writeBaseIDOrNone(M.BaseSelectorID, M.LocalNumSelectors);
}
@@ -7382,12 +7372,8 @@ void ASTWriter::ReaderInitialized(ASTReader *Reader) {
Chain = Reader;
- // Note, this will get called multiple times, once one the reader starts up
- // and again each time it's done reading a PCH or module.
- FirstMacroID = NUM_PREDEF_MACRO_IDS + Chain->getTotalNumMacros();
FirstSubmoduleID = NUM_PREDEF_SUBMODULE_IDS + Chain->getTotalNumSubmodules();
FirstSelectorID = NUM_PREDEF_SELECTOR_IDS + Chain->getTotalNumSelectors();
- NextMacroID = FirstMacroID;
NextSelectorID = FirstSelectorID;
NextSubmoduleID = FirstSubmoduleID;
}
@@ -7415,6 +7401,14 @@ void ASTWriter::IdentifierRead(IdentifierID ID, IdentifierInfo *II) {
void ASTWriter::MacroRead(serialization::MacroID ID, MacroInfo *MI) {
// Always keep the hi...
[truncated]
|
ChuanqiXu9
force-pushed
the
RefactoringMacroID
branch
from
68df09c to
70559cb
Compare
jansvoboda11
reviewed
Nov 4, 2025
Contributor
jansvoboda11
left a comment
jansvoboda11
left a comment
There was a problem hiding this comment.
This is great, thanks for working on this. I left mostly small style suggestions, but I'd really appreciate stronger typing of MacroID and friend.
Contributor
|
Also, can you please compile real-world modules (e.g. libc++) and show how this affects the PCM size? |
ChuanqiXu9
force-pushed
the
RefactoringMacroID
branch
from
70559cb to
09f3de8
Compare
Member
Author
In my simple test, it didn't affect the PCM size. It makes sense in the theory too. Since on the one hand, the number of MacroID and PreprocessedEntityID is much less than other IDs. And also, we didn't use VBR encoding for MacroID and PreprocessedEntityID in my memory. So it might not affect the PCM size significantly. |
Contributor
That's why I'm specifically asking for a real-world workload.
That really depends on the code.
For both |
ChuanqiXu9
force-pushed
the
RefactoringMacroID
branch
from
09f3de8 to
5c491d0
Compare
Member
Author
Yeah, I tried clang std module for libc++ and it shows some size inflation and it goes away after I store MacroID into two slots manually for And when I reading code, I found |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar to previous no transitive changes to decls, types, identifiers and source locations (
#92083
#92085
#92511
#86912
)
This patch does the same thing for MacroID and PreprocessedEntityID.
Some background
Previously we record different IDs linearly. That is, when writing a module, if we have 17 decls in imported modules, the ID of decls in the module will start from 18. This makes the contents of the BMI changes if the we add/remove any decls, types, identifiers and source locations in the imported modules.
This makes it hard for us to reduce recompilations with modules. We want to skip recompilations as we think the modules can help us to remove fake dependencies. This can be done by split the ID into <ModuleIndex, LocalIndex> pairs.
This is ALREADY done for several different ID above. We call it non-casacading changes (https://clang.llvm.org/docs/StandardCPlusPlusModules.html#experimental-non-cascading-changes). Our internal users have already used this feature and it works well for years.
Now we want to extend this to MacroID and PreprocessedEntityID. This is helpful for us in the downstream as we allowed named modules to export macros. But I believe this is also helpful for header-like modules if you'd like to explore the area.
And also I think this is a nice cleanup too.
Given the use of MacroID and PreprocessedEntityID are not as complicated as other IDs in the above series, I feel the patch itself should be good. I hope the vendors can test the patch to make sure it won't affect existing users.