Merge pull request #8599 from akyrtzi/akyrtzi/pr/ignore-profile-flags-for-pch-release

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

Author: bnbarham

clang/include/clang/Basic/DiagnosticCASKinds.td

@@ -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;

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

@@ -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

clang/lib/Frontend/CompilerInvocation.cpp

@@ -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);

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -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;
 

clang/lib/Tooling/DependencyScanning/IncludeTreeActionController.cpp

@@ -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());
@@ -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)

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

@@ -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();
@@ -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.
@@ -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();

clang/lib/Tooling/DependencyScanning/ScanAndUpdateArgs.cpp

@@ -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.

clang/test/CAS/modules-include-tree-pgo-option.c

@@ -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();
+}

clang/test/CAS/pgo-profile-with-pch.c

@@ -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

clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template

@@ -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"
   }
 ]

clang/test/ClangScanDeps/removed-args.c

@@ -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
@@ -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"
@@ -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"
@@ -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"
+  }
+]