Author: Chuanqi Xu Date: 2024-06-21T09:21:40+08:00 New Revision: 03921b979d67657bfc9cf8240add2484cc4df6a7
URL: https://github.com/llvm/llvm-project/commit/03921b979d67657bfc9cf8240add2484cc4df6a7 DIFF: https://github.com/llvm/llvm-project/commit/03921b979d67657bfc9cf8240add2484cc4df6a7.diff LOG: [serialization] No transitive type change (#92511) Following of https://github.com/llvm/llvm-project/pull/92085. #### motivation The motivation is still cutting of the unnecessary change in the dependency chain. See the above link (recursively) for details. And this will be the last patch of the `no-transitive-*-change` series. If there are any following patches, they might be C++20 Named modules specific to handle special grammars like `ADL` (See the reply in https://discourse.llvm.org/t/rfc-c-20-modules-introduce-thin-bmi-and-decls-hash/74755/53 for example). So they won't affect the whole serialization part as the series patch did. #### example After this patch, finally we are able to cut of unnecessary change of types. For example, ``` //--- m-partA.cppm export module m:partA; //--- m-partA.v1.cppm export module m:partA; namespace NS { class A { public: int getValue() { return 43; } }; } //--- m-partB.cppm export module m:partB; export inline int getB() { return 430; } //--- m.cppm export module m; export import :partA; export import :partB; //--- useBOnly.cppm export module useBOnly; import m; export inline int get() { return getB(); } ``` The BMI of `useBOnly.cppm` is expected to not change if we only add a new class in `m:partA`. This will be pretty useful in practice. #### implementation details The key idea of this patch is similar with the previous patches: extend the 32bits type ID to 64bits so that we can store the module file index in the higher bits. Then the encoding of the type ID is independent on the imported modules. But there are two differences from the previous patches: - TypeID is not completely an index of serialized types. We used the lower 3 bits to store the qualifiers. - TypeID won't take part in any lookup process. So the uses of TypeID is much less than the previous patches. The first difference make we have some more slightly complex bit operations. And the second difference makes the patch much simpler than the previous ones. Added: clang/test/Modules/no-transitive-type-change.cppm Modified: clang/include/clang/Serialization/ASTBitCodes.h clang/include/clang/Serialization/ASTReader.h clang/include/clang/Serialization/ASTRecordReader.h clang/include/clang/Serialization/ModuleFile.h clang/lib/Serialization/ASTCommon.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/ModuleFile.cpp clang/test/Modules/no-transitive-decls-change.cppm clang/test/Modules/no-transitive-identifier-change.cppm Removed: ################################################################################ diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 316350d779e90..38502a23f805e 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -26,6 +26,7 @@ #include "clang/Serialization/SourceLocationEncoding.h" #include "llvm/ADT/DenseMapInfo.h" #include "llvm/Bitstream/BitCodes.h" +#include "llvm/Support/MathExtras.h" #include <cassert> #include <cstdint> @@ -70,41 +71,64 @@ using DeclID = DeclIDBase::DeclID; /// An ID number that refers to a type in an AST file. /// -/// The ID of a type is partitioned into two parts: the lower -/// three bits are used to store the const/volatile/restrict -/// qualifiers (as with QualType) and the upper bits provide a -/// type index. The type index values are partitioned into two +/// The ID of a type is partitioned into three parts: +/// - the lower three bits are used to store the const/volatile/restrict +/// qualifiers (as with QualType). +/// - the next 29 bits provide a type index in the corresponding +/// module file. +/// - the upper 32 bits provide a module file index. +/// +/// The type index values are partitioned into two /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a -/// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are -/// other types that have serialized representations. -using TypeID = uint32_t; +/// placeholder for "no type". The module file index for predefined +/// types are always 0 since they don't belong to any modules. +/// Values from NUM_PREDEF_TYPE_IDs are other types that have +/// serialized representations. +using TypeID = uint64_t; +/// Same with TypeID except that the LocalTypeID is only meaningful +/// with the corresponding ModuleFile. +/// +/// FIXME: Make TypeID and LocalTypeID a class to improve the type +/// safety. +using LocalTypeID = TypeID; /// A type index; the type ID with the qualifier bits removed. +/// Keep structure alignment 32-bit since the blob is assumed as 32-bit +/// aligned. class TypeIdx { + uint32_t ModuleFileIndex = 0; uint32_t Idx = 0; public: TypeIdx() = default; - explicit TypeIdx(uint32_t index) : Idx(index) {} - uint32_t getIndex() const { return Idx; } + explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx) + : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {} + + uint32_t getModuleFileIndex() const { return ModuleFileIndex; } + + uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; } TypeID asTypeID(unsigned FastQuals) const { if (Idx == uint32_t(-1)) return TypeID(-1); - return (Idx << Qualifiers::FastWidth) | FastQuals; + unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals; + return ((uint64_t)ModuleFileIndex << 32) | Index; } static TypeIdx fromTypeID(TypeID ID) { if (ID == TypeID(-1)) - return TypeIdx(-1); + return TypeIdx(0, -1); - return TypeIdx(ID >> Qualifiers::FastWidth); + return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes<TypeID>(32)) >> + Qualifiers::FastWidth); } }; +static_assert(alignof(TypeIdx) == 4); + /// A structure for putting "fast"-unqualified QualTypes into a /// DenseMap. This uses the standard pointer hash function. struct UnsafeQualTypeDenseMapInfo { diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 0e95d82928459..f41c473c97cd9 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -491,14 +491,6 @@ class ASTReader /// ID = (I + 1) << FastQual::Width has already been loaded llvm::PagedVector<QualType> TypesLoaded; - using GlobalTypeMapType = - ContinuousRangeMap<serialization::TypeID, ModuleFile *, 4>; - - /// Mapping from global type IDs to the module in which the - /// type resides along with the offset that should be added to the - /// global type ID to produce a local ID. - GlobalTypeMapType GlobalTypeMap; - /// Declarations that have already been loaded from the chain. /// /// When the pointer at index I is non-NULL, the declaration with ID @@ -1429,8 +1421,8 @@ class ASTReader RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {} }; - QualType readTypeRecord(unsigned Index); - RecordLocation TypeCursorForIndex(unsigned Index); + QualType readTypeRecord(serialization::TypeID ID); + RecordLocation TypeCursorForIndex(serialization::TypeID ID); void LoadedDecl(unsigned Index, Decl *D); Decl *ReadDeclRecord(GlobalDeclID ID); void markIncompleteDeclChain(Decl *D); @@ -1544,6 +1536,11 @@ class ASTReader std::pair<ModuleFile *, unsigned> translateIdentifierIDToIndex(serialization::IdentifierID ID) const; + /// Translate an \param TypeID ID to the index of TypesLoaded + /// array and the corresponding module file. + std::pair<ModuleFile *, unsigned> + translateTypeIDToIndex(serialization::TypeID ID) const; + public: /// Load the AST file and validate its contents against the given /// Preprocessor. @@ -1892,10 +1889,11 @@ class ASTReader QualType GetType(serialization::TypeID ID); /// Resolve a local type ID within a given AST file into a type. - QualType getLocalType(ModuleFile &F, unsigned LocalID); + QualType getLocalType(ModuleFile &F, serialization::LocalTypeID LocalID); /// Map a local type ID within a given AST file into a global type ID. - serialization::TypeID getGlobalTypeID(ModuleFile &F, unsigned LocalID) const; + serialization::TypeID + getGlobalTypeID(ModuleFile &F, serialization::LocalTypeID LocalID) const; /// Read a type from the current position in the given record, which /// was read from the given AST file. diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h index d00fb182f05f4..2561418b78ca7 100644 --- a/clang/include/clang/Serialization/ASTRecordReader.h +++ b/clang/include/clang/Serialization/ASTRecordReader.h @@ -163,7 +163,7 @@ class ASTRecordReader void readTypeLoc(TypeLoc TL, LocSeq *Seq = nullptr); /// Map a local type ID within a given AST file to a global type ID. - serialization::TypeID getGlobalTypeID(unsigned LocalID) const { + serialization::TypeID getGlobalTypeID(serialization::TypeID LocalID) const { return Reader->getGlobalTypeID(*F, LocalID); } diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 3787f4eeb8a8b..3e920c0f68360 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -482,9 +482,6 @@ class ModuleFile { /// the global type ID space. serialization::TypeID BaseTypeIndex = 0; - /// Remapping table for type IDs in this module. - ContinuousRangeMap<uint32_t, int, 2> TypeRemap; - // === Miscellaneous === /// Diagnostic IDs and their mappings that the user changed. diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp index 3385cb8aad7e4..444a8a3d3a514 100644 --- a/clang/lib/Serialization/ASTCommon.cpp +++ b/clang/lib/Serialization/ASTCommon.cpp @@ -283,7 +283,7 @@ serialization::TypeIdxFromBuiltin(const BuiltinType *BT) { break; } - return TypeIdx(ID); + return TypeIdx(0, ID); } unsigned serialization::ComputeHash(Selector Sel) { diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 43013ab61474e..552a3af546e75 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3395,20 +3395,11 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, "duplicate TYPE_OFFSET record in AST file"); F.TypeOffsets = reinterpret_cast<const UnalignedUInt64 *>(Blob.data()); F.LocalNumTypes = Record[0]; - unsigned LocalBaseTypeIndex = Record[1]; F.BaseTypeIndex = getTotalNumTypes(); - if (F.LocalNumTypes > 0) { - // Introduce the global -> local mapping for types within this module. - GlobalTypeMap.insert(std::make_pair(getTotalNumTypes(), &F)); - - // Introduce the local -> global mapping for types within this module. - F.TypeRemap.insertOrReplace( - std::make_pair(LocalBaseTypeIndex, - F.BaseTypeIndex - LocalBaseTypeIndex)); - + if (F.LocalNumTypes > 0) TypesLoaded.resize(TypesLoaded.size() + F.LocalNumTypes); - } + break; } @@ -4084,7 +4075,6 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const { RemapBuilder PreprocessedEntityRemap(F.PreprocessedEntityRemap); RemapBuilder SubmoduleRemap(F.SubmoduleRemap); RemapBuilder SelectorRemap(F.SelectorRemap); - RemapBuilder TypeRemap(F.TypeRemap); auto &ImportedModuleVector = F.TransitiveImports; assert(ImportedModuleVector.empty()); @@ -4120,8 +4110,6 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const { endian::readNext<uint32_t, llvm::endianness::little>(Data); uint32_t SelectorIDOffset = endian::readNext<uint32_t, llvm::endianness::little>(Data); - uint32_t TypeIndexOffset = - endian::readNext<uint32_t, llvm::endianness::little>(Data); auto mapOffset = [&](uint32_t Offset, uint32_t BaseOffset, RemapBuilder &Remap) { @@ -4136,7 +4124,6 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const { PreprocessedEntityRemap); mapOffset(SubmoduleIDOffset, OM->BaseSubmoduleID, SubmoduleRemap); mapOffset(SelectorIDOffset, OM->BaseSelectorID, SelectorRemap); - mapOffset(TypeIndexOffset, OM->BaseTypeIndex, TypeRemap); } } @@ -5115,12 +5102,12 @@ void ASTReader::InitializeContext() { // Load the special types. if (SpecialTypes.size() >= NumSpecialTypeIDs) { - if (unsigned String = SpecialTypes[SPECIAL_TYPE_CF_CONSTANT_STRING]) { + if (TypeID String = SpecialTypes[SPECIAL_TYPE_CF_CONSTANT_STRING]) { if (!Context.CFConstantStringTypeDecl) Context.setCFConstantStringType(GetType(String)); } - if (unsigned File = SpecialTypes[SPECIAL_TYPE_FILE]) { + if (TypeID File = SpecialTypes[SPECIAL_TYPE_FILE]) { QualType FileType = GetType(File); if (FileType.isNull()) { Error("FILE type is NULL"); @@ -5141,7 +5128,7 @@ void ASTReader::InitializeContext() { } } - if (unsigned Jmp_buf = SpecialTypes[SPECIAL_TYPE_JMP_BUF]) { + if (TypeID Jmp_buf = SpecialTypes[SPECIAL_TYPE_JMP_BUF]) { QualType Jmp_bufType = GetType(Jmp_buf); if (Jmp_bufType.isNull()) { Error("jmp_buf type is NULL"); @@ -5162,7 +5149,7 @@ void ASTReader::InitializeContext() { } } - if (unsigned Sigjmp_buf = SpecialTypes[SPECIAL_TYPE_SIGJMP_BUF]) { + if (TypeID Sigjmp_buf = SpecialTypes[SPECIAL_TYPE_SIGJMP_BUF]) { QualType Sigjmp_bufType = GetType(Sigjmp_buf); if (Sigjmp_bufType.isNull()) { Error("sigjmp_buf type is NULL"); @@ -5180,25 +5167,24 @@ void ASTReader::InitializeContext() { } } - if (unsigned ObjCIdRedef - = SpecialTypes[SPECIAL_TYPE_OBJC_ID_REDEFINITION]) { + if (TypeID ObjCIdRedef = SpecialTypes[SPECIAL_TYPE_OBJC_ID_REDEFINITION]) { if (Context.ObjCIdRedefinitionType.isNull()) Context.ObjCIdRedefinitionType = GetType(ObjCIdRedef); } - if (unsigned ObjCClassRedef - = SpecialTypes[SPECIAL_TYPE_OBJC_CLASS_REDEFINITION]) { + if (TypeID ObjCClassRedef = + SpecialTypes[SPECIAL_TYPE_OBJC_CLASS_REDEFINITION]) { if (Context.ObjCClassRedefinitionType.isNull()) Context.ObjCClassRedefinitionType = GetType(ObjCClassRedef); } - if (unsigned ObjCSelRedef - = SpecialTypes[SPECIAL_TYPE_OBJC_SEL_REDEFINITION]) { + if (TypeID ObjCSelRedef = + SpecialTypes[SPECIAL_TYPE_OBJC_SEL_REDEFINITION]) { if (Context.ObjCSelRedefinitionType.isNull()) Context.ObjCSelRedefinitionType = GetType(ObjCSelRedef); } - if (unsigned Ucontext_t = SpecialTypes[SPECIAL_TYPE_UCONTEXT_T]) { + if (TypeID Ucontext_t = SpecialTypes[SPECIAL_TYPE_UCONTEXT_T]) { QualType Ucontext_tType = GetType(Ucontext_t); if (Ucontext_tType.isNull()) { Error("ucontext_t type is NULL"); @@ -6683,10 +6669,8 @@ void ASTReader::ReadPragmaDiagnosticMappings(DiagnosticsEngine &Diag) { } /// Get the correct cursor and offset for loading a type. -ASTReader::RecordLocation ASTReader::TypeCursorForIndex(unsigned Index) { - GlobalTypeMapType::iterator I = GlobalTypeMap.find(Index); - assert(I != GlobalTypeMap.end() && "Corrupted global type map"); - ModuleFile *M = I->second; +ASTReader::RecordLocation ASTReader::TypeCursorForIndex(TypeID ID) { + auto [M, Index] = translateTypeIDToIndex(ID); return RecordLocation(M, M->TypeOffsets[Index - M->BaseTypeIndex].get() + M->DeclsBlockStartOffset); } @@ -6707,10 +6691,10 @@ static std::optional<Type::TypeClass> getTypeClassForCode(TypeCode code) { /// routine actually reads the record corresponding to the type at the given /// location. It is a helper routine for GetType, which deals with reading type /// IDs. -QualType ASTReader::readTypeRecord(unsigned Index) { +QualType ASTReader::readTypeRecord(TypeID ID) { assert(ContextObj && "reading type with no AST context"); ASTContext &Context = *ContextObj; - RecordLocation Loc = TypeCursorForIndex(Index); + RecordLocation Loc = TypeCursorForIndex(ID); BitstreamCursor &DeclsCursor = Loc.F->DeclsCursor; // Keep track of where we are in the stream, then jump back there @@ -7151,15 +7135,44 @@ TypeSourceInfo *ASTRecordReader::readTypeSourceInfo() { return TInfo; } +static unsigned getIndexForTypeID(serialization::TypeID ID) { + return (ID & llvm::maskTrailingOnes<TypeID>(32)) >> Qualifiers::FastWidth; +} + +static unsigned getModuleFileIndexForTypeID(serialization::TypeID ID) { + return ID >> 32; +} + +static bool isPredefinedType(serialization::TypeID ID) { + // We don't need to erase the higher bits since if these bits are not 0, + // it must be larger than NUM_PREDEF_TYPE_IDS. + return (ID >> Qualifiers::FastWidth) < NUM_PREDEF_TYPE_IDS; +} + +std::pair<ModuleFile *, unsigned> +ASTReader::translateTypeIDToIndex(serialization::TypeID ID) const { + assert(!isPredefinedType(ID) && + "Predefined type shouldn't be in TypesLoaded"); + unsigned ModuleFileIndex = getModuleFileIndexForTypeID(ID); + assert(ModuleFileIndex && "Untranslated Local Decl?"); + + ModuleFile *OwningModuleFile = &getModuleManager()[ModuleFileIndex - 1]; + assert(OwningModuleFile && + "untranslated type ID or local type ID shouldn't be in TypesLoaded"); + + return {OwningModuleFile, + OwningModuleFile->BaseTypeIndex + getIndexForTypeID(ID)}; +} + QualType ASTReader::GetType(TypeID ID) { assert(ContextObj && "reading type with no AST context"); ASTContext &Context = *ContextObj; unsigned FastQuals = ID & Qualifiers::FastMask; - unsigned Index = ID >> Qualifiers::FastWidth; - if (Index < NUM_PREDEF_TYPE_IDS) { + if (isPredefinedType(ID)) { QualType T; + unsigned Index = getIndexForTypeID(ID); switch ((PredefinedTypeIDs)Index) { case PREDEF_TYPE_LAST_ID: // We should never use this one. @@ -7432,10 +7445,11 @@ QualType ASTReader::GetType(TypeID ID) { return T.withFastQualifiers(FastQuals); } - Index -= NUM_PREDEF_TYPE_IDS; + unsigned Index = translateTypeIDToIndex(ID).second; + assert(Index < TypesLoaded.size() && "Type index out-of-range"); if (TypesLoaded[Index].isNull()) { - TypesLoaded[Index] = readTypeRecord(Index); + TypesLoaded[Index] = readTypeRecord(ID); if (TypesLoaded[Index].isNull()) return QualType(); @@ -7448,27 +7462,28 @@ QualType ASTReader::GetType(TypeID ID) { return TypesLoaded[Index].withFastQualifiers(FastQuals); } -QualType ASTReader::getLocalType(ModuleFile &F, unsigned LocalID) { +QualType ASTReader::getLocalType(ModuleFile &F, LocalTypeID LocalID) { return GetType(getGlobalTypeID(F, LocalID)); } -serialization::TypeID -ASTReader::getGlobalTypeID(ModuleFile &F, unsigned LocalID) const { - unsigned FastQuals = LocalID & Qualifiers::FastMask; - unsigned LocalIndex = LocalID >> Qualifiers::FastWidth; - - if (LocalIndex < NUM_PREDEF_TYPE_IDS) +serialization::TypeID ASTReader::getGlobalTypeID(ModuleFile &F, + LocalTypeID LocalID) const { + if (isPredefinedType(LocalID)) return LocalID; if (!F.ModuleOffsetMap.empty()) ReadModuleOffsetMap(F); - ContinuousRangeMap<uint32_t, int, 2>::iterator I - = F.TypeRemap.find(LocalIndex - NUM_PREDEF_TYPE_IDS); - assert(I != F.TypeRemap.end() && "Invalid index into type index remap"); + unsigned ModuleFileIndex = getModuleFileIndexForTypeID(LocalID); + LocalID &= llvm::maskTrailingOnes<TypeID>(32); + + if (ModuleFileIndex == 0) + LocalID -= NUM_PREDEF_TYPE_IDS << Qualifiers::FastWidth; - unsigned GlobalIndex = LocalIndex + I->second; - return (GlobalIndex << Qualifiers::FastWidth) | FastQuals; + ModuleFile &MF = + ModuleFileIndex ? *F.TransitiveImports[ModuleFileIndex - 1] : F; + ModuleFileIndex = MF.Index + 1; + return ((uint64_t)ModuleFileIndex << 32) | LocalID; } TemplateArgumentLocInfo @@ -8224,7 +8239,6 @@ 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 type map", GlobalTypeMap); dumpModuleIDMap("Global macro map", GlobalMacroMap); dumpModuleIDMap("Global submodule map", GlobalSubmoduleMap); dumpModuleIDMap("Global selector map", GlobalSelectorMap); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 346bab34acb5e..0297e20e9116f 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3270,17 +3270,18 @@ void ASTWriter::WritePragmaDiagnosticMappings(const DiagnosticsEngine &Diag, /// Write the representation of a type to the AST stream. void ASTWriter::WriteType(QualType T) { TypeIdx &IdxRef = TypeIdxs[T]; - if (IdxRef.getIndex() == 0) // we haven't seen this type before. - IdxRef = TypeIdx(NextTypeID++); + if (IdxRef.getValue() == 0) // we haven't seen this type before. + IdxRef = TypeIdx(0, NextTypeID++); TypeIdx Idx = IdxRef; - assert(Idx.getIndex() >= FirstTypeID && "Re-writing a type from a prior AST"); + assert(Idx.getModuleFileIndex() == 0 && "Re-writing a type from a prior AST"); + assert(Idx.getValue() >= FirstTypeID && "Writing predefined type"); // Emit the type's representation. uint64_t Offset = ASTTypeWriter(*this).write(T) - DeclTypesBlockStartOffset; // Record the offset for this type. - unsigned Index = Idx.getIndex() - FirstTypeID; + uint64_t Index = Idx.getValue() - FirstTypeID; if (TypeOffsets.size() == Index) TypeOffsets.emplace_back(Offset); else if (TypeOffsets.size() < Index) { @@ -3353,12 +3354,10 @@ void ASTWriter::WriteTypeDeclOffsets() { auto Abbrev = std::make_shared<BitCodeAbbrev>(); Abbrev->Add(BitCodeAbbrevOp(TYPE_OFFSET)); Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // # of types - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // base type index Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // types block unsigned TypeOffsetAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); { - RecordData::value_type Record[] = {TYPE_OFFSET, TypeOffsets.size(), - FirstTypeID - NUM_PREDEF_TYPE_IDS}; + RecordData::value_type Record[] = {TYPE_OFFSET, TypeOffsets.size()}; Stream.EmitRecordWithBlob(TypeOffsetAbbrev, Record, bytes(TypeOffsets)); } @@ -5464,7 +5463,6 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, M.NumPreprocessedEntities); writeBaseIDOrNone(M.BaseSubmoduleID, M.LocalNumSubmodules); writeBaseIDOrNone(M.BaseSelectorID, M.LocalNumSelectors); - writeBaseIDOrNone(M.BaseTypeIndex, M.LocalNumTypes); } } RecordData::value_type Record[] = {MODULE_OFFSET_MAP}; @@ -6138,9 +6136,9 @@ static TypeID MakeTypeID(ASTContext &Context, QualType T, return TypeIdxFromBuiltin(BT).asTypeID(FastQuals); if (T == Context.AutoDeductTy) - return TypeIdx(PREDEF_TYPE_AUTO_DEDUCT).asTypeID(FastQuals); + return TypeIdx(0, PREDEF_TYPE_AUTO_DEDUCT).asTypeID(FastQuals); if (T == Context.AutoRRefDeductTy) - return TypeIdx(PREDEF_TYPE_AUTO_RREF_DEDUCT).asTypeID(FastQuals); + return TypeIdx(0, PREDEF_TYPE_AUTO_RREF_DEDUCT).asTypeID(FastQuals); return IdxForType(T).asTypeID(FastQuals); } @@ -6153,7 +6151,7 @@ TypeID ASTWriter::GetOrCreateTypeID(QualType T) { assert(!T.getLocalFastQualifiers()); TypeIdx &Idx = TypeIdxs[T]; - if (Idx.getIndex() == 0) { + if (Idx.getValue() == 0) { if (DoneWritingDeclsAndTypes) { assert(0 && "New type seen after serializing all the types to emit!"); return TypeIdx(); @@ -6161,7 +6159,7 @@ TypeID ASTWriter::GetOrCreateTypeID(QualType T) { // We haven't seen this type before. Assign it a new ID and put it // into the queue of types to emit. - Idx = TypeIdx(NextTypeID++); + Idx = TypeIdx(0, NextTypeID++); DeclTypesToEmit.push(T); } return Idx; @@ -6658,11 +6656,9 @@ void ASTWriter::ReaderInitialized(ASTReader *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. - FirstTypeID = NUM_PREDEF_TYPE_IDS + Chain->getTotalNumTypes(); FirstMacroID = NUM_PREDEF_MACRO_IDS + Chain->getTotalNumMacros(); FirstSubmoduleID = NUM_PREDEF_SUBMODULE_IDS + Chain->getTotalNumSubmodules(); FirstSelectorID = NUM_PREDEF_SELECTOR_IDS + Chain->getTotalNumSelectors(); - NextTypeID = FirstTypeID; NextMacroID = FirstMacroID; NextSelectorID = FirstSelectorID; NextSubmoduleID = FirstSubmoduleID; @@ -6691,13 +6687,24 @@ void ASTWriter::MacroRead(serialization::MacroID ID, MacroInfo *MI) { } void ASTWriter::TypeRead(TypeIdx Idx, QualType T) { - // Always take the highest-numbered type index. This copes with an interesting + // Always take the type index that comes in later module files. + // This copes with an interesting // case for chained AST writing where we schedule writing the type and then, // later, deserialize the type from another AST. In this case, we want to - // keep the higher-numbered entry so that we can properly write it out to + // keep the entry from a later module so that we can properly write it out to // the AST file. TypeIdx &StoredIdx = TypeIdxs[T]; - if (Idx.getIndex() >= StoredIdx.getIndex()) + + // Ignore it if the type comes from the current being written module file. + // Since the current module file being written logically has the highest + // index. + unsigned ModuleFileIndex = StoredIdx.getModuleFileIndex(); + if (ModuleFileIndex == 0 && StoredIdx.getValue()) + return; + + // Otherwise, keep the highest ID since the module file comes later has + // higher module file indexes. + if (Idx.getModuleFileIndex() >= StoredIdx.getModuleFileIndex()) StoredIdx = Idx; } diff --git a/clang/lib/Serialization/ModuleFile.cpp b/clang/lib/Serialization/ModuleFile.cpp index 7976c28b28671..4858cdbda5545 100644 --- a/clang/lib/Serialization/ModuleFile.cpp +++ b/clang/lib/Serialization/ModuleFile.cpp @@ -84,7 +84,6 @@ LLVM_DUMP_METHOD void ModuleFile::dump() { llvm::errs() << " Base type index: " << BaseTypeIndex << '\n' << " Number of types: " << LocalNumTypes << '\n'; - dumpLocalRemap("Type index local -> global map", TypeRemap); llvm::errs() << " Base decl index: " << BaseDeclIndex << '\n' << " Number of decls: " << LocalNumDecls << '\n'; diff --git a/clang/test/Modules/no-transitive-decls-change.cppm b/clang/test/Modules/no-transitive-decls-change.cppm index 42ac061bc90b3..83594b09ea789 100644 --- a/clang/test/Modules/no-transitive-decls-change.cppm +++ b/clang/test/Modules/no-transitive-decls-change.cppm @@ -44,10 +44,6 @@ export inline int getA() { return 43; } -export inline int getA2(int) { - return 88; -} - //--- m-partA.v1.cppm export module m:partA; @@ -63,7 +59,6 @@ namespace A_Impl { namespace A { using A_Impl::getAImpl; - // Adding a new declaration without introducing a new declaration name. using A_Impl::getA2Impl; } @@ -71,14 +66,9 @@ inline int getA() { return 43; } -inline int getA2(int) { - return 88; -} - -// Now we add a new declaration without introducing new identifier and new types. // The consuming module which didn't use m:partA completely is expected to be // not changed. -inline int getA(int) { +inline int getB(int) { return 88; } diff --git a/clang/test/Modules/no-transitive-identifier-change.cppm b/clang/test/Modules/no-transitive-identifier-change.cppm index 97e8ac4444fdd..541c8ae754960 100644 --- a/clang/test/Modules/no-transitive-identifier-change.cppm +++ b/clang/test/Modules/no-transitive-identifier-change.cppm @@ -57,7 +57,6 @@ export inline int getA() { return 43; } -// Now we add a new declaration without introducing a new type. // The consuming module which didn't use m:partA completely is expected to be // not changed. export inline int getA2() { @@ -67,7 +66,7 @@ export inline int getA2() { export class A { public: int getMem(); - // Now we add a new declaration without introducing a new type. + // The consuming module which didn't use m:partA completely is expected to be // not changed. int getMem2(); @@ -77,7 +76,6 @@ export template <typename T> class ATempl { public: T getT(); - // Add a new declaration without introducing a new type. T getT2(); }; diff --git a/clang/test/Modules/no-transitive-type-change.cppm b/clang/test/Modules/no-transitive-type-change.cppm new file mode 100644 index 0000000000000..9ec72e851b17d --- /dev/null +++ b/clang/test/Modules/no-transitive-type-change.cppm @@ -0,0 +1,80 @@ +// Testing that changing a type in an unused module file won't change +// the BMI of the current module file. +// +// RUN: rm -rf %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/m-partA.cppm -emit-reduced-module-interface -o %t/m-partA.pcm +// RUN: %clang_cc1 -std=c++20 %t/m-partA.v1.cppm -emit-reduced-module-interface -o \ +// RUN: %t/m-partA.v1.pcm +// RUN: %clang_cc1 -std=c++20 %t/m-partB.cppm -emit-reduced-module-interface -o %t/m-partB.pcm +// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-reduced-module-interface -o %t/m.pcm \ +// RUN: -fmodule-file=m:partA=%t/m-partA.pcm -fmodule-file=m:partB=%t/m-partB.pcm +// RUN: %clang_cc1 -std=c++20 %t/m.cppm -emit-reduced-module-interface -o %t/m.v1.pcm \ +// RUN: -fmodule-file=m:partA=%t/m-partA.v1.pcm -fmodule-file=m:partB=%t/m-partB.pcm +// +// RUN: %clang_cc1 -std=c++20 %t/useBOnly.cppm -emit-reduced-module-interface -o %t/useBOnly.pcm \ +// RUN: -fmodule-file=m=%t/m.pcm -fmodule-file=m:partA=%t/m-partA.pcm \ +// RUN: -fmodule-file=m:partB=%t/m-partB.pcm +// RUN: %clang_cc1 -std=c++20 %t/useBOnly.cppm -emit-reduced-module-interface -o %t/useBOnly.v1.pcm \ +// RUN: -fmodule-file=m=%t/m.v1.pcm -fmodule-file=m:partA=%t/m-partA.v1.pcm \ +// RUN: -fmodule-file=m:partB=%t/m-partB.pcm +// Since useBOnly only uses partB from module M, the change in partA shouldn't affect +// useBOnly. +// RUN: diff %t/useBOnly.pcm %t/useBOnly.v1.pcm &> /dev/null +// +// RUN: %clang_cc1 -std=c++20 %t/useAOnly.cppm -emit-reduced-module-interface -o %t/useAOnly.pcm \ +// RUN: -fmodule-file=m=%t/m.pcm -fmodule-file=m:partA=%t/m-partA.pcm \ +// RUN: -fmodule-file=m:partB=%t/m-partB.pcm +// RUN: %clang_cc1 -std=c++20 %t/useAOnly.cppm -emit-reduced-module-interface -o %t/useAOnly.v1.pcm \ +// RUN: -fmodule-file=m=%t/m.v1.pcm -fmodule-file=m:partA=%t/m-partA.v1.pcm \ +// RUN: -fmodule-file=m:partB=%t/m-partB.pcm +// Since useAOnly uses partA from module M, the change in partA should affect useAOnly. +// RUN: not diff %t/useAOnly.pcm %t/useAOnly.v1.pcm &> /dev/null + +//--- m-partA.cppm +export module m:partA; + +export int getValueFromA() { return 43; } + +//--- m-partA.v1.cppm +export module m:partA; + +export int getValueFromA() { return 43; } + +namespace NS { + class A { + public: + int getValue() { + return 43; + } + }; +} + +//--- m-partB.cppm +export module m:partB; + +export inline int getB() { + return 430; +} + +//--- m.cppm +export module m; +export import :partA; +export import :partB; + +//--- useBOnly.cppm +export module useBOnly; +import m; + +export inline int get() { + return getB(); +} + +//--- useAOnly.cppm +export module useAOnly; +import m; + +export inline int get() { + return getValueFromA(); +} _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
