jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, rsmith, Bigcheese.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
With implicit modules, it's impossible to load a PCM file that was built using
different command-line macro definitions. This is guaranteed by the fact that
they contribute to the context hash. This means that we don't need to store
those macros into PCM files for validation purposes. This patch avoids
serializing them in those circumstances, since there's no other use for
command-line macro definitions (besides "-module-file-info").
For a typical Apple project, this speeds up the dependency scan by 5.6% and
shrinks the cache with scanning PCMs by 26%.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158136
Files:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/GeneratePCH.cpp
clang/test/Modules/module_file_info.m
Index: clang/test/Modules/module_file_info.m
===================================================================
--- clang/test/Modules/module_file_info.m
+++ clang/test/Modules/module_file_info.m
@@ -42,9 +42,6 @@
// CHECK: Preprocessor options:
// CHECK: Uses compiler/target-specific predefines [-undef]: Yes
// CHECK: Uses detailed preprocessing record (for indexing): No
-// CHECK: Predefined macros:
-// CHECK: -DBLARG
-// CHECK: -DWIBBLE=WOBBLE
// CHECK: Input file: {{.*}}module.map
// CHECK-NEXT: Input file: {{.*}}module_private.map
// CHECK-NEXT: Input file: {{.*}}DependsOnModule.h
Index: clang/lib/Serialization/GeneratePCH.cpp
===================================================================
--- clang/lib/Serialization/GeneratePCH.cpp
+++ clang/lib/Serialization/GeneratePCH.cpp
@@ -25,11 +25,11 @@
StringRef OutputFile, StringRef isysroot, std::shared_ptr<PCHBuffer> Buffer,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
bool AllowASTWithErrors, bool IncludeTimestamps,
- bool ShouldCacheASTInMemory)
+ bool BuildingImplicitModule, bool ShouldCacheASTInMemory)
: PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
Writer(Stream, this->Buffer->Data, ModuleCache, Extensions,
- IncludeTimestamps),
+ IncludeTimestamps, BuildingImplicitModule),
AllowASTWithErrors(AllowASTWithErrors),
ShouldCacheASTInMemory(ShouldCacheASTInMemory) {
this->Buffer->IsComplete = false;
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1466,11 +1466,19 @@
Record.clear();
const PreprocessorOptions &PPOpts = PP.getPreprocessorOpts();
- // Macro definitions.
- Record.push_back(PPOpts.Macros.size());
- for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
- AddString(PPOpts.Macros[I].first, Record);
- Record.push_back(PPOpts.Macros[I].second);
+ // If we're building an implicit module with a context hash, the importer is
+ // guaranteed to have the same macros defined on the command line. Skip
+ // writing them.
+ bool SkipMacros = BuildingImplicitModule && !HSOpts.DisableModuleHash;
+ bool WriteMacros = !SkipMacros;
+ Record.push_back(WriteMacros);
+ if (WriteMacros) {
+ // Macro definitions.
+ Record.push_back(PPOpts.Macros.size());
+ for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
+ AddString(PPOpts.Macros[I].first, Record);
+ Record.push_back(PPOpts.Macros[I].second);
+ }
}
// Includes
@@ -4539,9 +4547,10 @@
SmallVectorImpl<char> &Buffer,
InMemoryModuleCache &ModuleCache,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
- bool IncludeTimestamps)
+ bool IncludeTimestamps, bool BuildingImplicitModule)
: Stream(Stream), Buffer(Buffer), ModuleCache(ModuleCache),
- IncludeTimestamps(IncludeTimestamps) {
+ IncludeTimestamps(IncludeTimestamps),
+ BuildingImplicitModule(BuildingImplicitModule) {
for (const auto &Ext : Extensions) {
if (auto Writer = Ext->createExtensionWriter(*this))
ModuleFileExtensionWriters.push_back(std::move(Writer));
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -208,11 +208,12 @@
}
bool ChainedASTReaderListener::ReadPreprocessorOptions(
- const PreprocessorOptions &PPOpts, bool Complain,
+ const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) {
- return First->ReadPreprocessorOptions(PPOpts, Complain,
+ return First->ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
SuggestedPredefines) ||
- Second->ReadPreprocessorOptions(PPOpts, Complain, SuggestedPredefines);
+ Second->ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
+ SuggestedPredefines);
}
void ChainedASTReaderListener::ReadCounter(const serialization::ModuleFile &M,
@@ -658,92 +659,94 @@
/// are no differences in the options between the two.
static bool checkPreprocessorOptions(
const PreprocessorOptions &PPOpts,
- const PreprocessorOptions &ExistingPPOpts, DiagnosticsEngine *Diags,
- FileManager &FileMgr, std::string &SuggestedPredefines,
- const LangOptions &LangOpts,
+ const PreprocessorOptions &ExistingPPOpts, bool ReadMacros,
+ DiagnosticsEngine *Diags, FileManager &FileMgr,
+ std::string &SuggestedPredefines, const LangOptions &LangOpts,
OptionValidation Validation = OptionValidateContradictions) {
- // Check macro definitions.
- MacroDefinitionsMap ASTFileMacros;
- collectMacroDefinitions(PPOpts, ASTFileMacros);
- MacroDefinitionsMap ExistingMacros;
- SmallVector<StringRef, 4> ExistingMacroNames;
- collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
-
- // Use a line marker to enter the <command line> file, as the defines and
- // undefines here will have come from the command line.
- SuggestedPredefines += "# 1 \"<command line>\" 1\n";
-
- for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
- // Dig out the macro definition in the existing preprocessor options.
- StringRef MacroName = ExistingMacroNames[I];
- std::pair<StringRef, bool> Existing = ExistingMacros[MacroName];
-
- // Check whether we know anything about this macro name or not.
- llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>::iterator Known =
- ASTFileMacros.find(MacroName);
- if (Validation == OptionValidateNone || Known == ASTFileMacros.end()) {
- if (Validation == OptionValidateStrictMatches) {
- // If strict matches are requested, don't tolerate any extra defines on
- // the command line that are missing in the AST file.
+ if (ReadMacros) {
+ // Check macro definitions.
+ MacroDefinitionsMap ASTFileMacros;
+ collectMacroDefinitions(PPOpts, ASTFileMacros);
+ MacroDefinitionsMap ExistingMacros;
+ SmallVector<StringRef, 4> ExistingMacroNames;
+ collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
+
+ // Use a line marker to enter the <command line> file, as the defines and
+ // undefines here will have come from the command line.
+ SuggestedPredefines += "# 1 \"<command line>\" 1\n";
+
+ for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
+ // Dig out the macro definition in the existing preprocessor options.
+ StringRef MacroName = ExistingMacroNames[I];
+ std::pair<StringRef, bool> Existing = ExistingMacros[MacroName];
+
+ // Check whether we know anything about this macro name or not.
+ llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>::iterator Known =
+ ASTFileMacros.find(MacroName);
+ if (Validation == OptionValidateNone || Known == ASTFileMacros.end()) {
+ if (Validation == OptionValidateStrictMatches) {
+ // If strict matches are requested, don't tolerate any extra defines on
+ // the command line that are missing in the AST file.
+ if (Diags) {
+ Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true;
+ }
+ return true;
+ }
+ // FIXME: Check whether this identifier was referenced anywhere in the
+ // AST file. If so, we should reject the AST file. Unfortunately, this
+ // information isn't in the control block. What shall we do about it?
+
+ if (Existing.second) {
+ SuggestedPredefines += "#undef ";
+ SuggestedPredefines += MacroName.str();
+ SuggestedPredefines += '\n';
+ } else {
+ SuggestedPredefines += "#define ";
+ SuggestedPredefines += MacroName.str();
+ SuggestedPredefines += ' ';
+ SuggestedPredefines += Existing.first.str();
+ SuggestedPredefines += '\n';
+ }
+ continue;
+ }
+
+ // If the macro was defined in one but undef'd in the other, we have a
+ // conflict.
+ if (Existing.second != Known->second.second) {
if (Diags) {
- Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true;
+ Diags->Report(diag::err_pch_macro_def_undef)
+ << MacroName << Known->second.second;
}
return true;
}
- // FIXME: Check whether this identifier was referenced anywhere in the
- // AST file. If so, we should reject the AST file. Unfortunately, this
- // information isn't in the control block. What shall we do about it?
-
- if (Existing.second) {
- SuggestedPredefines += "#undef ";
- SuggestedPredefines += MacroName.str();
- SuggestedPredefines += '\n';
- } else {
- SuggestedPredefines += "#define ";
- SuggestedPredefines += MacroName.str();
- SuggestedPredefines += ' ';
- SuggestedPredefines += Existing.first.str();
- SuggestedPredefines += '\n';
+
+ // If the macro was #undef'd in both, or if the macro bodies are identical,
+ // it's fine.
+ if (Existing.second || Existing.first == Known->second.first) {
+ ASTFileMacros.erase(Known);
+ continue;
}
- continue;
- }
- // If the macro was defined in one but undef'd in the other, we have a
- // conflict.
- if (Existing.second != Known->second.second) {
+ // The macro bodies differ; complain.
if (Diags) {
- Diags->Report(diag::err_pch_macro_def_undef)
- << MacroName << Known->second.second;
+ Diags->Report(diag::err_pch_macro_def_conflict)
+ << MacroName << Known->second.first << Existing.first;
}
return true;
}
- // If the macro was #undef'd in both, or if the macro bodies are identical,
- // it's fine.
- if (Existing.second || Existing.first == Known->second.first) {
- ASTFileMacros.erase(Known);
- continue;
- }
+ // Leave the <command line> file and return to <built-in>.
+ SuggestedPredefines += "# 1 \"<built-in>\" 2\n";
- // The macro bodies differ; complain.
- if (Diags) {
- Diags->Report(diag::err_pch_macro_def_conflict)
- << MacroName << Known->second.first << Existing.first;
- }
- return true;
- }
-
- // Leave the <command line> file and return to <built-in>.
- SuggestedPredefines += "# 1 \"<built-in>\" 2\n";
-
- if (Validation == OptionValidateStrictMatches) {
- // If strict matches are requested, don't tolerate any extra defines in
- // the AST file that are missing on the command line.
- for (const auto &MacroName : ASTFileMacros.keys()) {
- if (Diags) {
- Diags->Report(diag::err_pch_macro_def_undef) << MacroName << false;
+ if (Validation == OptionValidateStrictMatches) {
+ // If strict matches are requested, don't tolerate any extra defines in
+ // the AST file that are missing on the command line.
+ for (const auto &MacroName : ASTFileMacros.keys()) {
+ if (Diags) {
+ Diags->Report(diag::err_pch_macro_def_undef) << MacroName << false;
+ }
+ return true;
}
- return true;
}
}
@@ -805,11 +808,11 @@
}
bool PCHValidator::ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) {
const PreprocessorOptions &ExistingPPOpts = PP.getPreprocessorOpts();
- return checkPreprocessorOptions(PPOpts, ExistingPPOpts,
+ return checkPreprocessorOptions(PPOpts, ExistingPPOpts, ReadMacros,
Complain? &Reader.Diags : nullptr,
PP.getFileManager(),
SuggestedPredefines,
@@ -817,12 +820,12 @@
}
bool SimpleASTReaderListener::ReadPreprocessorOptions(
- const PreprocessorOptions &PPOpts,
- bool Complain,
- std::string &SuggestedPredefines) {
- return checkPreprocessorOptions(PPOpts, PP.getPreprocessorOpts(), nullptr,
- PP.getFileManager(), SuggestedPredefines,
- PP.getLangOpts(), OptionValidateNone);
+ const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain,
+ std::string &SuggestedPredefines) {
+ return checkPreprocessorOptions(PPOpts, PP.getPreprocessorOpts(), ReadMacros,
+ nullptr, PP.getFileManager(),
+ SuggestedPredefines, PP.getLangOpts(),
+ OptionValidateNone);
}
/// Check the header search options deserialized from the control block
@@ -5274,10 +5277,10 @@
}
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
return checkPreprocessorOptions(
- PPOpts, ExistingPPOpts, /*Diags=*/nullptr, FileMgr,
+ PPOpts, ExistingPPOpts, ReadMacros, /*Diags=*/nullptr, FileMgr,
SuggestedPredefines, ExistingLangOpts,
StrictOptionMatches ? OptionValidateStrictMatches
: OptionValidateContradictions);
@@ -6040,18 +6043,20 @@
return Listener.ReadHeaderSearchPaths(HSOpts, Complain);
}
-bool ASTReader::ParsePreprocessorOptions(const RecordData &Record,
- bool Complain,
+bool ASTReader::ParsePreprocessorOptions(const RecordData &Record, bool Complain,
ASTReaderListener &Listener,
std::string &SuggestedPredefines) {
PreprocessorOptions PPOpts;
unsigned Idx = 0;
// Macro definitions/undefs
- for (unsigned N = Record[Idx++]; N; --N) {
- std::string Macro = ReadString(Record, Idx);
- bool IsUndef = Record[Idx++];
- PPOpts.Macros.push_back(std::make_pair(Macro, IsUndef));
+ bool ReadMacros = Record[Idx++];
+ if (ReadMacros) {
+ for (unsigned N = Record[Idx++]; N; --N) {
+ std::string Macro = ReadString(Record, Idx);
+ bool IsUndef = Record[Idx++];
+ PPOpts.Macros.push_back(std::make_pair(Macro, IsUndef));
+ }
}
// Includes
@@ -6070,7 +6075,7 @@
PPOpts.ObjCXXARCStandardLibrary =
static_cast<ObjCXXARCStandardLibraryKind>(Record[Idx++]);
SuggestedPredefines.clear();
- return Listener.ReadPreprocessorOptions(PPOpts, Complain,
+ return Listener.ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
SuggestedPredefines);
}
Index: clang/lib/Frontend/FrontendActions.cpp
===================================================================
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -141,7 +141,8 @@
CI.getPreprocessor(), CI.getModuleCache(), OutputFile, Sysroot, Buffer,
FrontendOpts.ModuleFileExtensions,
CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
- FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
+ FrontendOpts.IncludeTimestamps, FrontendOpts.BuildingImplicitModule,
+ +CI.getLangOpts().CacheGeneratedPCH));
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
CI, std::string(InFile), OutputFile, std::move(OS), Buffer));
@@ -204,6 +205,7 @@
/*IncludeTimestamps=*/
+CI.getFrontendOpts().BuildingImplicitModule &&
+CI.getFrontendOpts().IncludeTimestamps,
+ /*BuildingImplicitModule=*/+CI.getFrontendOpts().BuildingImplicitModule,
/*ShouldCacheASTInMemory=*/
+CI.getFrontendOpts().BuildingImplicitModule));
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
@@ -660,7 +662,7 @@
}
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
Out.indent(2) << "Preprocessor options:\n";
DUMP_BOOLEAN(PPOpts.UsePredefines,
@@ -668,7 +670,7 @@
DUMP_BOOLEAN(PPOpts.DetailedRecord,
"Uses detailed preprocessing record (for indexing)");
- if (!PPOpts.Macros.empty()) {
+ if (ReadMacros) {
Out.indent(4) << "Predefined macros:\n";
}
Index: clang/lib/Frontend/ASTUnit.cpp
===================================================================
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -581,7 +581,8 @@
return false;
}
- bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
+ bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
this->PPOpts = PPOpts;
return false;
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -143,6 +143,11 @@
/// file is up to date, but not otherwise.
bool IncludeTimestamps;
+ /// Indicates whether the AST file being written is an implicit module.
+ /// If that's the case, we may be able to skip writing some information that
+ /// are guaranteed to be the same in the importer by the context hash.
+ bool BuildingImplicitModule = false;
+
/// Indicates when the AST writing is actively performing
/// serialization, rather than just queueing updates.
bool WritingAST = false;
@@ -571,7 +576,7 @@
ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
InMemoryModuleCache &ModuleCache,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
- bool IncludeTimestamps = true);
+ bool IncludeTimestamps = true, bool BuildingImplicitModule = false);
~ASTWriter() override;
ASTContext &getASTContext() const {
@@ -809,6 +814,7 @@
std::shared_ptr<PCHBuffer> Buffer,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
bool AllowASTWithErrors = false, bool IncludeTimestamps = true,
+ bool BuildingImplicitModule = false,
bool ShouldCacheASTInMemory = false);
~PCHGenerator() override;
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -199,7 +199,7 @@
/// \returns true to indicate the preprocessor options are invalid, or false
/// otherwise.
virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) {
return false;
}
@@ -274,7 +274,7 @@
StringRef SpecificModuleCachePath,
bool Complain) override;
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override;
void ReadCounter(const serialization::ModuleFile &M, unsigned Value) override;
@@ -304,7 +304,8 @@
bool AllowCompatibleDifferences) override;
bool ReadDiagnosticOptions(IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts,
bool Complain) override;
- bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
+ bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override;
bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef SpecificModuleCachePath,
@@ -325,7 +326,8 @@
public:
SimpleASTReaderListener(Preprocessor &PP) : PP(PP) {}
- bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
+ bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
