Skip to content

[6.0][clang][cas] Don't include PGO-related files in the include-tree when they are not used #8599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions clang/include/clang/Basic/DiagnosticCASKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ def err_cas_cannot_be_initialized : Error<
def err_cas_cannot_parse_root_id : Error<
"CAS cannot parse root-id '%0' specified by -fcas-fs">, DefaultFatal;
def err_cas_filesystem_cannot_be_initialized : Error<
"CAS filesystem cannot be initialized from root-id '%0' specified by"
" -fcas-fs">, DefaultFatal;
"CAS filesystem cannot be initialized from root-id '%0': %1">, DefaultFatal;
def err_cas_filesystem_cannot_set_working_directory : Error<
"CAS filesystem cannot set working directory to '%0' specified by"
" -fcas-fs-working-directory">, DefaultFatal;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,11 @@ class ModuleDepCollector final : public DependencyCollector {
ModuleDeps &Deps);
};

/// Resets codegen options that don't affect modules/PCH.
void resetBenignCodeGenOptions(frontend::ActionKind ProgramAction,
const LangOptions &LangOpts,
CodeGenOptions &CGOpts);

} // end namespace dependencies
} // end namespace tooling
} // end namespace clang
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1552,9 +1552,8 @@ createBaseFS(const FileSystemOptions &FSOpts, const FrontendOptions &FEOpts,
auto ExpectedFS = IsIncludeTreeFS ? makeIncludeTreeFS(std::move(CAS), *RootID)
: makeCASFS(std::move(CAS), *RootID);
if (!ExpectedFS) {
llvm::consumeError(ExpectedFS.takeError());
Diags.Report(diag::err_cas_filesystem_cannot_be_initialized)
<< RootIDString;
<< RootIDString << llvm::toString(ExpectedFS.takeError());
return makeEmptyCASFS();
}
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = std::move(*ExpectedFS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,6 @@ void DependencyScanningWorker::computeDependenciesFromCompilerInvocation(

// Adjust the invocation.
auto &Frontend = Invocation->getFrontendOpts();
Frontend.ProgramAction = frontend::RunPreprocessorOnly;
Frontend.OutputFile = "/dev/null";
Frontend.DisableFree = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ Error IncludeTreeActionController::finalizeModuleBuild(
CompilerInstance &ModuleScanInstance) {
// FIXME: the scan invocation is incorrect here; we need the `NewInvocation`
// from `finalizeModuleInvocation` to finish the tree.
resetBenignCodeGenOptions(
frontend::GenerateModule,
ModuleScanInstance.getInvocation().getLangOpts(),
ModuleScanInstance.getInvocation().getCodeGenOpts());
auto Builder = BuilderStack.pop_back_val();
auto Tree = Builder->finishIncludeTree(ModuleScanInstance,
ModuleScanInstance.getInvocation());
Expand Down Expand Up @@ -589,6 +593,8 @@ IncludeTreeBuilder::finishIncludeTree(CompilerInstance &ScanInstance,

auto addFile = [&](StringRef FilePath,
bool IgnoreFileError = false) -> Error {
if (FilePath.empty())
return Error::success();
llvm::ErrorOr<const FileEntry *> FE = FM.getFile(FilePath);
if (!FE) {
if (IgnoreFileError)
Expand Down
33 changes: 24 additions & 9 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,26 @@ void ModuleDepCollector::addOutputPaths(CowCompilerInvocation &CI,
}
}

void dependencies::resetBenignCodeGenOptions(frontend::ActionKind ProgramAction,
const LangOptions &LangOpts,
CodeGenOptions &CGOpts) {
// TODO: Figure out better way to set options to their default value.
if (ProgramAction == frontend::GenerateModule) {
CGOpts.MainFileName.clear();
CGOpts.DwarfDebugFlags.clear();
}
if (ProgramAction == frontend::GeneratePCH ||
(ProgramAction == frontend::GenerateModule && !LangOpts.ModulesCodegen)) {
CGOpts.DebugCompilationDir.clear();
CGOpts.CoverageCompilationDir.clear();
CGOpts.CoverageDataFile.clear();
CGOpts.CoverageNotesFile.clear();
CGOpts.ProfileInstrumentUsePath.clear();
CGOpts.SampleProfileFile.clear();
CGOpts.ProfileRemappingFile.clear();
}
}

static CowCompilerInvocation
makeCommonInvocationForModuleBuild(CompilerInvocation CI) {
CI.resetNonModularOptions();
Expand All @@ -176,15 +196,8 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) {
// LLVM options are not going to affect the AST
CI.getFrontendOpts().LLVMArgs.clear();

// TODO: Figure out better way to set options to their default value.
CI.getCodeGenOpts().MainFileName.clear();
CI.getCodeGenOpts().DwarfDebugFlags.clear();
if (!CI.getLangOpts().ModulesCodegen) {
CI.getCodeGenOpts().DebugCompilationDir.clear();
CI.getCodeGenOpts().CoverageCompilationDir.clear();
CI.getCodeGenOpts().CoverageDataFile.clear();
CI.getCodeGenOpts().CoverageNotesFile.clear();
}
resetBenignCodeGenOptions(frontend::GenerateModule, CI.getLangOpts(),
CI.getCodeGenOpts());

// Map output paths that affect behaviour to "-" so their existence is in the
// context hash. The final path will be computed in addOutputPaths.
Expand Down Expand Up @@ -368,6 +381,8 @@ static bool needsModules(FrontendInputFile FIF) {

void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
CI.clearImplicitModuleBuildOptions();
resetBenignCodeGenOptions(CI.getFrontendOpts().ProgramAction,
CI.getLangOpts(), CI.getCodeGenOpts());

if (llvm::any_of(CI.getFrontendOpts().Inputs, needsModules)) {
Preprocessor &PP = ScanInstance.getPreprocessor();
Expand Down
8 changes: 2 additions & 6 deletions clang/lib/Tooling/DependencyScanning/ScanAndUpdateArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,8 @@ void tooling::dependencies::configureInvocationForCaching(
// Clear this otherwise it defeats the purpose of making the compilation key
// independent of certain arguments.
CI.getCodeGenOpts().DwarfDebugFlags.clear();
if (FrontendOpts.ProgramAction == frontend::GeneratePCH) {
// Clear paths that are emitted into binaries; they do not affect PCH.
// For modules this is handled in ModuleDepCollector.
CI.getCodeGenOpts().CoverageDataFile.clear();
CI.getCodeGenOpts().CoverageNotesFile.clear();
}
resetBenignCodeGenOptions(FrontendOpts.ProgramAction, CI.getLangOpts(),
CI.getCodeGenOpts());

HeaderSearchOptions &HSOpts = CI.getHeaderSearchOpts();
// Avoid writing potentially volatile diagnostic options into pcms.
Expand Down
42 changes: 42 additions & 0 deletions clang/test/CAS/modules-include-tree-pgo-option.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// REQUIRES: ondisk_cas

// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json

// RUN: llvm-profdata merge -o %t/instrumentation.profdata %S/Inputs/pgo.profraw
// RUN: clang-scan-deps -compilation-database %t/cdb.json \
// RUN: -cas-path %t/cas -module-files-dir %t/outputs \
// RUN: -format experimental-include-tree-full -mode preprocess-dependency-directives \
// RUN: > %t/deps.json

// RUN: %deps-to-rsp %t/deps.json --module-name Top > %t/Top.rsp
// RUN: FileCheck %s --input-file=%t/Top.rsp

// RUN: cat %t/Top.rsp | sed -E 's|.*"-fcas-include-tree" "(llvmcas://[[:xdigit:]]+)".*|\1|' > %t/Top.casid
// RUN: clang-cas-test -cas %t/cas -print-include-tree @%t/Top.casid | FileCheck %s
// CHECK-NOT: instrumentation.profdata

//--- cdb.json.template
[{
"file": "DIR/tu.m",
"directory": "DIR",
"command": "clang -fsyntax-only DIR/tu.m -I DIR -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache -Rcompile-job-cache -fprofile-instr-use=DIR/instrumentation.profdata"
}]

//--- module.modulemap
module Top { header "Top.h" export *}

//--- Top.h
#pragma once
struct Top {
int x;
};
void top(void);

//--- tu.m
#import "Top.h"

void tu(void) {
top();
}
28 changes: 28 additions & 0 deletions clang/test/CAS/pgo-profile-with-pch.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// RUN: rm -rf %t.dir && mkdir -p %t.dir

// Check that use of profile data for PCH is ignored

// RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgo.profraw
// RUN: %clang -cc1depscan -fdepscan=inline -fdepscan-include-tree -o %t-pch.rsp -cc1-args -cc1 -triple x86_64-apple-macosx12.0.0 -emit-pch -O3 -Rcompile-job-cache \
// RUN: -x c-header %s -o %t.h.pch -fcas-path %t.dir/cas -fprofile-instrument-use-path=%t.profdata
// RUN: %clang @%t-pch.rsp 2>&1 | FileCheck %s --check-prefix=CACHE-MISS
// RUN: FileCheck %s -check-prefix=PCHPROF -input-file %t-pch.rsp
// PCHPROF-NOT: -fprofile-instrument-use-path

// Update profdata file contents
// RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgo2.profraw

// Use the modified profdata file for the main file along with the PCH.
// RUN: %clang -cc1depscan -fdepscan=inline -fdepscan-include-tree -o %t.rsp -cc1-args -cc1 -triple x86_64-apple-macosx12.0.0 -emit-obj -O3 -Rcompile-job-cache \
// RUN: -x c %s -o %t.o -fcas-path %t.dir/cas -fprofile-instrument-use-path=%t.profdata -include-pch %t.h.pch
// RUN: %clang @%t.rsp 2>&1 | FileCheck %s --check-prefix=CACHE-MISS
// RUN: FileCheck %s -check-prefix=TUPROF -input-file %t.rsp
// TUPROF: -fprofile-instrument-use-path

// Check that the modified profdata is ignored when re-scanning for the PCH.
// RUN: %clang -cc1depscan -fdepscan=inline -fdepscan-include-tree -o %t-pch2.rsp -cc1-args -cc1 -triple x86_64-apple-macosx12.0.0 -emit-pch -O3 -Rcompile-job-cache \
// RUN: -x c-header %s -o %t.h.pch -fcas-path %t.dir/cas -fprofile-instrument-use-path=%t.profdata
// RUN: diff -u %t-pch.rsp %t-pch2.rsp

// CACHE-MISS: remark: compile job cache miss
// CACHE-HIT: remark: compile job cache hit
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"directory": "DIR",
"command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-validate-once-per-build-session -fbuild-session-file=DIR/build-session -fmodules-prune-interval=123 -fmodules-prune-after=123 -fmodules-cache-path=DIR/cache -include DIR/header.h -grecord-command-line -fdebug-compilation-dir=DIR/debug -fcoverage-compilation-dir=DIR/coverage -ftest-coverage -o DIR/tu.o -serialize-diagnostics DIR/tu.diag -MT tu -MD -MF DIR/tu.d",
"command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-validate-once-per-build-session -fbuild-session-file=DIR/build-session -fmodules-prune-interval=123 -fmodules-prune-after=123 -fmodules-cache-path=DIR/cache -include DIR/header.h -grecord-command-line -fdebug-compilation-dir=DIR/debug -fcoverage-compilation-dir=DIR/coverage -ftest-coverage -fprofile-instr-use=DIR/tu.profdata -o DIR/tu.o -serialize-diagnostics DIR/tu.diag -MT tu -MD -MF DIR/tu.d",
"file": "DIR/tu.c"
}
]
32 changes: 32 additions & 0 deletions clang/test/ClangScanDeps/removed-args.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
// RUN: rm -rf %t && mkdir %t
// RUN: cp %S/Inputs/removed-args/* %t
// RUN: touch %t/build-session
// RUN: touch %t/tu.proftext
// RUN: llvm-profdata merge %t/tu.proftext -o %t/tu.profdata

// RUN: sed "s|DIR|%/t|g" %S/Inputs/removed-args/cdb.json.template > %t/cdb.json
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
Expand All @@ -25,6 +27,7 @@
// CHECK-NOT: "-fcoverage-compilation-dir="
// CHECK-NOT: "-coverage-notes-file
// CHECK-NOT: "-coverage-data-file
// CHECK-NOT: "-fprofile-instrument-use-path
// CHECK-NOT: "-dwarf-debug-flags"
// CHECK-NOT: "-main-file-name"
// CHECK-NOT: "-include"
Expand All @@ -50,6 +53,7 @@
// CHECK-NOT: "-fcoverage-compilation-dir=
// CHECK-NOT: "-coverage-notes-file
// CHECK-NOT: "-coverage-data-file
// CHECK-NOT: "-fprofile-instrument-use-path
// CHECK-NOT: "-dwarf-debug-flags"
// CHECK-NOT: "-main-file-name"
// CHECK-NOT: "-include"
Expand Down Expand Up @@ -89,3 +93,31 @@
// CHECK-NOT: "-fmodules-prune-interval=
// CHECK-NOT: "-fmodules-prune-after=
// CHECK: ],

// Check for removed args for PCH invocations.

// RUN: split-file %s %t
// RUN: sed "s|DIR|%/t|g" %t/cdb-pch.json.template > %t/cdb-pch.json
// RUN: clang-scan-deps -compilation-database %t/cdb-pch.json -format experimental-full > %t/result-pch.json
// RUN: cat %t/result-pch.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=PCH
//
// PCH-NOT: "-fdebug-compilation-dir="
// PCH-NOT: "-fcoverage-compilation-dir="
// PCH-NOT: "-coverage-notes-file
// PCH-NOT: "-coverage-data-file
// PCH-NOT: "-fprofile-instrument-use-path
// PCH-NOT: "-include"
// PCH-NOT: "-fmodules-cache-path=
// PCH-NOT: "-fmodules-validate-once-per-build-session"
// PCH-NOT: "-fbuild-session-timestamp=
// PCH-NOT: "-fmodules-prune-interval=
// PCH-NOT: "-fmodules-prune-after=

//--- cdb-pch.json.template
[
{
"directory": "DIR",
"command": "clang -x c-header DIR/header.h -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -fdebug-compilation-dir=DIR/debug -fcoverage-compilation-dir=DIR/coverage -ftest-coverage -fprofile-instr-use=DIR/tu.profdata -o DIR/header.h.pch -serialize-diagnostics DIR/header.h.pch.diag ",
"file": "DIR/header.h.pch"
}
]