Author: Jan Svoboda Date: 2023-07-17T13:50:23-07:00 New Revision: abcf7ce45794839a473a236d55d163016cde5ba6
URL: https://github.com/llvm/llvm-project/commit/abcf7ce45794839a473a236d55d163016cde5ba6 DIFF: https://github.com/llvm/llvm-project/commit/abcf7ce45794839a473a236d55d163016cde5ba6.diff LOG: [clang][modules] Serialize `Module::DefinitionLoc` This is a prep patch for avoiding the quadratic number of calls to `HeaderSearch::lookupModule()` in `ASTReader` for each (transitively) loaded PCM file. (Specifically in the context of `clang-scan-deps`). This patch explicitly serializes `Module::DefinitionLoc` so that we can stop relying on it being filled by the module map parser. This change also required change to the module map parser, where we used the absence of `DefinitionLoc` to determine whether a file came from a PCM file. We also need to make sure we consider the "containing" module map affecting when writing a PCM, so that it's not stripped during serialization, which ensures `DefinitionLoc` still ends up pointing to the correct offset. This is intended to be a NFC change. Reviewed By: benlangmuir Differential Revision: https://reviews.llvm.org/D150292 Added: Modified: clang/include/clang/Serialization/ASTBitCodes.h clang/lib/Lex/ModuleMap.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 074d1002913084..2ae9e09998c4c1 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -41,7 +41,7 @@ namespace serialization { /// Version 4 of AST files also requires that the version control branch and /// revision match exactly, since there is no backward compatibility of /// AST files at this time. -const unsigned VERSION_MAJOR = 26; +const unsigned VERSION_MAJOR = 27; /// AST file minor version number supported by this version of /// Clang. diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index c480719abc3679..c97642e5c6c87f 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -2016,10 +2016,12 @@ void ModuleMapParser::parseModuleDecl() { Module *ShadowingModule = nullptr; if (Module *Existing = Map.lookupModuleQualified(ModuleName, ActiveModule)) { // We might see a (re)definition of a module that we already have a - // definition for in two cases: + // definition for in three cases: // - If we loaded one definition from an AST file and we've just found a // corresponding definition in a module map file, or - bool LoadedFromASTFile = Existing->DefinitionLoc.isInvalid(); + bool LoadedFromASTFile = Existing->IsFromModuleFile; + // - If we previously inferred this module from diff erent module map file. + bool Inferred = Existing->IsInferred; // - If we're building a (preprocessed) module and we've just loaded the // module map file from which it was created. bool ParsedAsMainInput = @@ -2027,7 +2029,7 @@ void ModuleMapParser::parseModuleDecl() { Map.LangOpts.CurrentModule == ModuleName && SourceMgr.getDecomposedLoc(ModuleNameLoc).first != SourceMgr.getDecomposedLoc(Existing->DefinitionLoc).first; - if (!ActiveModule && (LoadedFromASTFile || ParsedAsMainInput)) { + if (!ActiveModule && (LoadedFromASTFile || Inferred || ParsedAsMainInput)) { // Skip the module definition. skipUntil(MMToken::RBrace); if (Tok.is(MMToken::RBrace)) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 380d117acc4973..0671f791530dd4 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5607,7 +5607,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, break; case SUBMODULE_DEFINITION: { - if (Record.size() < 12) + if (Record.size() < 13) return llvm::createStringError(std::errc::illegal_byte_sequence, "malformed module definition"); @@ -5616,6 +5616,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, SubmoduleID GlobalID = getGlobalSubmoduleID(F, Record[Idx++]); SubmoduleID Parent = getGlobalSubmoduleID(F, Record[Idx++]); Module::ModuleKind Kind = (Module::ModuleKind)Record[Idx++]; + SourceLocation DefinitionLoc = ReadSourceLocation(F, Record[Idx++]); bool IsFramework = Record[Idx++]; bool IsExplicit = Record[Idx++]; bool IsSystem = Record[Idx++]; @@ -5636,8 +5637,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, ModMap.findOrCreateModule(Name, ParentModule, IsFramework, IsExplicit) .first; - // FIXME: set the definition loc for CurrentModule, or call - // ModMap.setInferredModuleAllowedBy() + // FIXME: Call ModMap.setInferredModuleAllowedBy() SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS; if (GlobalIndex >= SubmodulesLoaded.size() || @@ -5666,6 +5666,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, } CurrentModule->Kind = Kind; + CurrentModule->DefinitionLoc = DefinitionLoc; CurrentModule->Signature = F.Signature; CurrentModule->IsFromModuleFile = true; CurrentModule->IsSystem = IsSystem || CurrentModule->IsSystem; diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 2ef0d09f359ac1..26279d399b53a9 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -200,7 +200,9 @@ std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP, CB(F); FileID FID = SourceMgr.translateFile(F); SourceLocation Loc = SourceMgr.getIncludeLoc(FID); - while (Loc.isValid()) { + // The include location of inferred module maps can point into the header + // file that triggered the inferring. Cut off the walk if that's the case. + while (Loc.isValid() && isModuleMap(SourceMgr.getFileCharacteristic(Loc))) { FID = SourceMgr.getFileID(Loc); CB(*SourceMgr.getFileEntryRefForID(FID)); Loc = SourceMgr.getIncludeLoc(FID); @@ -209,11 +211,18 @@ std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP, auto ProcessModuleOnce = [&](const Module *M) { for (const Module *Mod = M; Mod; Mod = Mod->Parent) - if (ProcessedModules.insert(Mod).second) + if (ProcessedModules.insert(Mod).second) { + auto Insert = [&](FileEntryRef F) { ModuleMaps.insert(F); }; + // The containing module map is affecting, because it's being pointed + // into by Module::DefinitionLoc. + if (auto ModuleMapFile = MM.getContainingModuleMapFile(Mod)) + ForIncludeChain(*ModuleMapFile, Insert); + // For inferred modules, the module map that allowed inferring is not in + // the include chain of the virtual containing module map file. It did + // affect the compilation, though. if (auto ModuleMapFile = MM.getModuleMapFileForUniquing(Mod)) - ForIncludeChain(*ModuleMapFile, [&](FileEntryRef F) { - ModuleMaps.insert(F); - }); + ForIncludeChain(*ModuleMapFile, Insert); + } }; for (const Module *CurrentModule : ModulesToProcess) { @@ -2687,6 +2696,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ID Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Parent Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 4)); // Kind + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // Definition location Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsExplicit Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsSystem @@ -2787,12 +2797,16 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { ParentID = SubmoduleIDs[Mod->Parent]; } + uint64_t DefinitionLoc = + SourceLocationEncoding::encode(getAdjustedLocation(Mod->DefinitionLoc)); + // Emit the definition of the block. { RecordData::value_type Record[] = {SUBMODULE_DEFINITION, ID, ParentID, (RecordData::value_type)Mod->Kind, + DefinitionLoc, Mod->IsFramework, Mod->IsExplicit, Mod->IsSystem, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
