[clang][cas] Remove -MT and -MP from the cache key

Pull the dependency options that only affect the output but not the list
of filenames out of the cache key, and apply them instead when replaying
the list of dependencies from the cache. The dependency file target
name(s) are typically an output path, so we want to avoid it showing up
in the cache key.

For now, the options that *do* affect the list of dependencies like
the difference between -MMD and -MD are still part of the key so that we
do not store many unnecessary system paths if they will not be used. It
would be straightforward to change that if it turned out to be
worthwhile by storing flags like IsSystem, IsModuleFile, etc.

Author: benlangmuir

clang/include/clang/Basic/DiagnosticCASKinds.td

@@ -23,6 +23,7 @@ def err_cas_depscan_daemon_connection: Error<
   "Failed to establish connection with depscan daemon: %0">, DefaultFatal;
 def err_cas_depscan_failed: Error<
   "CAS-based dependency scan failed: %0">, DefaultFatal;
+def err_cas_store: Error<"failed to store to CAS: %0">, DefaultFatal;
 
 def warn_clang_cache_disabled_caching: Warning<
   "caching disabled because %0">,

clang/include/clang/Frontend/CASDependencyCollector.h

@@ -0,0 +1,42 @@
+//===- CASDependencyCollector.h ---------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_FRONTEND_CASDEPENDENCYCOLLECTOR_H
+#define LLVM_CLANG_FRONTEND_CASDEPENDENCYCOLLECTOR_H
+
+#include "clang/Frontend/Utils.h"
+
+namespace llvm::cas {
+class CASOutputBackend;
+}
+
+namespace clang {
+
+/// Collects dependencies when attached to a Preprocessor (for includes) and
+/// ASTReader (for module imports), and writes it to the CAS in a manner
+/// suitable to be replayed into a DependencyFileGenerator.
+class CASDependencyCollector : public DependencyFileGenerator {
+public:
+  CASDependencyCollector(
+      const DependencyOutputOptions &Opts,
+      IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> OutputBackend);
+
+  static llvm::Error replay(const DependencyOutputOptions &Opts,
+                            cas::CASDB &CAS, cas::ObjectRef DepsRef,
+                            llvm::raw_ostream &OS);
+
+private:
+  void finishedMainFile(DiagnosticsEngine &Diags) override;
+
+  IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> CASOutputs;
+  std::string OutputName;
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_FRONTEND_CASDEPENDENCYCOLLECTOR_H

clang/include/clang/Frontend/CompilerInstance.h

@@ -21,6 +21,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/CAS/CASOutputBackend.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/VirtualOutputBackend.h"
@@ -92,6 +93,10 @@ class CompilerInstance : public ModuleLoader {
   /// The output context.
   IntrusiveRefCntPtr<llvm::vfs::OutputBackend> TheOutputBackend;
 
+  /// The underlying CAS output context, if any. Used to create CAS-specific
+  /// outputs.
+  IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> CASOutputBackend;
+
   /// The source manager.
   IntrusiveRefCntPtr<SourceManager> SourceMgr;
 
@@ -416,14 +421,20 @@ class CompilerInstance : public ModuleLoader {
   /// Set the output manager.
   void setOutputBackend(IntrusiveRefCntPtr<llvm::vfs::OutputBackend> NewOutputs);
 
+  void setCASOutputBackend(
+      clang::IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> NewOutputs);
+
   /// Create an output manager.
   void createOutputBackend();
 
   bool hasOutputBackend() const { return bool(TheOutputBackend); }
+  bool hasCASOutputBackend() const { return bool(CASOutputBackend); }
 
   llvm::vfs::OutputBackend &getOutputBackend();
   llvm::vfs::OutputBackend &getOrCreateOutputBackend();
 
+  llvm::cas::CASOutputBackend &getCASOutputBackend();
+
   /// Get the CAS, or create it using the configuration in CompilerInvocation.
   llvm::cas::CASDB &getOrCreateCAS();
 

clang/lib/Frontend/CASDependencyCollector.cpp

@@ -0,0 +1,73 @@
+//===--- CASDependencyCollector.cpp ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Frontend/CASDependencyCollector.h"
+#include "clang/Basic/DiagnosticCAS.h"
+#include "llvm/CAS/CASDB.h"
+#include "llvm/CAS/CASOutputBackend.h"
+#include "llvm/Support/VirtualOutputBackends.h"
+
+using namespace clang;
+using namespace clang::cas;
+
+CASDependencyCollector::CASDependencyCollector(
+    const DependencyOutputOptions &Opts,
+    IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> OutputBackend)
+    : DependencyFileGenerator(Opts, llvm::vfs::makeNullOutputBackend()),
+      CASOutputs(std::move(OutputBackend)), OutputName(Opts.OutputFile) {}
+
+llvm::Error CASDependencyCollector::replay(const DependencyOutputOptions &Opts,
+                                           CASDB &CAS, ObjectRef DepsRef,
+                                           llvm::raw_ostream &OS) {
+  auto Refs = CAS.load(DepsRef);
+  if (!Refs)
+    return Refs.takeError();
+
+  CASDependencyCollector DC(Opts, nullptr);
+  auto Err = CAS.forEachRef(*Refs, [&](ObjectRef Ref) -> llvm::Error {
+    auto PathHandle = CAS.load(Ref);
+    if (!PathHandle)
+      return PathHandle.takeError();
+    StringRef Path = CAS.getDataString(*PathHandle);
+    // This assumes the replay has the same filtering options as when it was
+    // originally computed. That avoids needing to store many unnecessary paths.
+    // FIXME: if a prefix map is enabled, we should remap the paths to the
+    // invocation's environment.
+    DC.addDependency(Path);
+    return llvm::Error::success();
+  });
+  if (Err)
+    return Err;
+
+  DC.outputDependencyFile(OS);
+  return llvm::Error::success();
+}
+
+void CASDependencyCollector::finishedMainFile(DiagnosticsEngine &Diags) {
+  CASDB &CAS = CASOutputs->getCAS();
+  ArrayRef<std::string> Files = getDependencies();
+  std::vector<ObjectRef> Refs;
+  Refs.reserve(Files.size());
+  for (StringRef File : Files) {
+    auto Handle = CAS.storeFromString({}, File);
+    if (!Handle) {
+      Diags.Report({}, diag::err_cas_store) << toString(Handle.takeError());
+      return;
+    }
+    Refs.push_back(CAS.getReference(*Handle));
+  }
+
+  auto Handle = CAS.store(Refs, {});
+  if (!Handle) {
+    Diags.Report({}, diag::err_cas_store) << toString(Handle.takeError());
+    return;
+  }
+
+  if (auto Err = CASOutputs->addObject(OutputName, CAS.getReference(*Handle)))
+    Diags.Report({}, diag::err_cas_store) << toString(std::move(Err));
+}

clang/lib/Frontend/CMakeLists.txt

@@ -13,6 +13,7 @@ add_clang_library(clangFrontend
   ASTConsumers.cpp
   ASTMerge.cpp
   ASTUnit.cpp
+  CASDependencyCollector.cpp
   ChainedDiagnosticConsumer.cpp
   ChainedIncludesSource.cpp
   CompileJobCacheKey.cpp

clang/lib/Frontend/CompileJobCacheKey.cpp

@@ -60,14 +60,20 @@ clang::createCompileJobCacheKey(CASDB &CAS, DiagnosticsEngine &Diags,
   CompilerInvocation InvocationForCacheKey(OriginalInvocation);
   FrontendOptions &FrontendOpts = InvocationForCacheKey.getFrontendOpts();
   DiagnosticOptions &DiagOpts = InvocationForCacheKey.getDiagnosticOpts();
+  DependencyOutputOptions &DepOpts =
+      InvocationForCacheKey.getDependencyOutputOpts();
   // Keep the key independent of the paths of these outputs.
   if (!FrontendOpts.OutputFile.empty())
     FrontendOpts.OutputFile = "-";
-  if (!InvocationForCacheKey.getDependencyOutputOpts().OutputFile.empty())
-    InvocationForCacheKey.getDependencyOutputOpts().OutputFile = "-";
+  if (!DepOpts.OutputFile.empty())
+    DepOpts.OutputFile = "-";
   // We always generate the serialized diagnostics so the key is independent of
   // the presence of '--serialize-diagnostics'.
   DiagOpts.DiagnosticSerializationFile.clear();
+  // Dependency options that do not affect the list of files are canonicalized.
+  if (!DepOpts.Targets.empty())
+    DepOpts.Targets = {"-"};
+  DepOpts.UsePhonyTargets = false;
 
   // Generate a new command-line in case Invocation has been canonicalized.
   llvm::BumpPtrAllocator Alloc;

clang/lib/Frontend/CompilerInstance.cpp

@@ -19,6 +19,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
+#include "clang/Frontend/CASDependencyCollector.h"
 #include "clang/Frontend/ChainedDiagnosticConsumer.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -506,9 +507,13 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
 
   // Handle generating dependencies, if requested.
   const DependencyOutputOptions &DepOpts = getDependencyOutputOpts();
-  if (!DepOpts.OutputFile.empty())
+  if (!DepOpts.OutputFile.empty()) {
+    if (hasCASOutputBackend())
+      addDependencyCollector(
+          std::make_shared<CASDependencyCollector>(DepOpts, CASOutputBackend));
     addDependencyCollector(
         std::make_shared<DependencyFileGenerator>(DepOpts, TheOutputBackend));
+  }
   if (!DepOpts.DOTOutputFile.empty())
     AttachDependencyGraphGen(*PP, DepOpts.DOTOutputFile,
                              getHeaderSearchOpts().Sysroot);
@@ -826,6 +831,11 @@ void CompilerInstance::setOutputBackend(
   assert(!TheOutputBackend && "Already has an output manager");
   TheOutputBackend = std::move(NewOutputs);
 }
+void CompilerInstance::setCASOutputBackend(
+    clang::IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> NewOutputs) {
+  assert(!CASOutputBackend && "Already has a CAS output backend");
+  CASOutputBackend = std::move(NewOutputs);
+}
 
 void CompilerInstance::createOutputBackend() {
   assert(!TheOutputBackend && "Already has an output manager");
@@ -843,6 +853,11 @@ llvm::vfs::OutputBackend &CompilerInstance::getOrCreateOutputBackend() {
   return getOutputBackend();
 }
 
+llvm::cas::CASOutputBackend &CompilerInstance::getCASOutputBackend() {
+  assert(CASOutputBackend);
+  return *CASOutputBackend;
+}
+
 llvm::cas::CASDB &CompilerInstance::getOrCreateCAS() {
   if (CAS)
     return *CAS;

clang/test/CAS/fcache-compile-job-dependency-file.c

@@ -4,20 +4,21 @@
 //
 // RUN: %clang -cc1 -triple x86_64-apple-macos11 \
 // RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
-// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o \
+// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o -isystem %t/sys \
 // RUN:   -dependency-file %t/deps1.d -MT depends 2>&1 \
 // RUN:   | FileCheck %s --allow-empty --check-prefix=CACHE-MISS
 //
 // RUN: FileCheck %s --input-file=%t/deps1.d --check-prefix=DEPS
 // DEPS: depends:
 // DEPS: main.c
 // DEPS: my_header.h
+// DEPS-NOT: sys.h
 
 // RUN: ls %t/output.o && rm %t/output.o
 //
 // RUN: %clang -cc1 -triple x86_64-apple-macos11 \
 // RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
-// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o \
+// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o -isystem %t/sys \
 // RUN:   -dependency-file %t/deps2.d -MT depends 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CACHE-HIT
 //
@@ -27,7 +28,38 @@
 // CACHE-HIT: remark: compile job cache hit
 // CACHE-MISS-NOT: remark: compile job cache hit
 
+// RUN: %clang -cc1 -triple x86_64-apple-macos11 \
+// RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
+// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o -isystem %t/sys \
+// RUN:   -dependency-file %t/deps3.d -MT other1 -MT other2 -MP 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CACHE-HIT
+
+// RUN: FileCheck %s --input-file=%t/deps3.d --check-prefix=DEPS_OTHER
+// DEPS_OTHER: other1 other2:
+// DEPS_OTHER: main.c
+// DEPS_OTHER: my_header.h
+// DEPS_OTHER-NOT: sys.h
+// DEPS_OTHER: my_header.h:
+
+// RUN: %clang -cc1 -triple x86_64-apple-macos11 \
+// RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
+// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o -isystem %t/sys \
+// RUN:   -sys-header-deps -dependency-file %t/deps4.d -MT depends 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CACHE-MISS
+
+// Note: currently options that affect the list of deps (like sys-header-deps)
+// are part of the cache key, to avoid saving unnecessary paths.
+
+// RUN: FileCheck %s --input-file=%t/deps4.d --check-prefix=DEPS_SYS
+// DEPS_SYS: depends:
+// DEPS_SYS: main.c
+// DEPS_SYS: my_header.h
+// DEPS_SYS: sys.h
+
 //--- main.c
 #include "my_header.h"
+#include <sys.h>
 
 //--- my_header.h
+
+//--- sys/sys.h

clang/tools/driver/cc1_main.cpp

@@ -20,6 +20,7 @@
 #include "clang/Config/config.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
+#include "clang/Frontend/CASDependencyCollector.h"
 #include "clang/Frontend/ChainedDiagnosticConsumer.h"
 #include "clang/Frontend/CompileJobCacheKey.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -470,6 +471,7 @@ Optional<int> CompileJobCache::tryReplayCachedResult(CompilerInstance &Clang) {
 
   // Set up the output backend so we can save / cache the result after.
   CASOutputs = llvm::makeIntrusiveRefCnt<llvm::cas::CASOutputBackend>(*CAS);
+  Clang.setCASOutputBackend(CASOutputs);
   for (OutputKind K : getAllOutputKinds()) {
     StringRef OutPath = getPathForOutputKind(K);
     if (!OutPath.empty())
@@ -682,6 +684,7 @@ Optional<int> CompileJobCache::replayCachedResult(CompilerInstance &Clang,
 
   for (size_t I = 0, E = Outputs->getNumReferences(); I + 1 < E; I += 2) {
     llvm::cas::CASID PathID = Outputs->getReferenceID(I);
+    cas::ObjectRef BytesRef = Outputs->getReference(I + 1);
     llvm::cas::CASID BytesID = Outputs->getReferenceID(I + 1);
 
     Optional<llvm::cas::ObjectProxy> PathProxy;
@@ -727,6 +730,12 @@ Optional<int> CompileJobCache::replayCachedResult(CompilerInstance &Clang,
       Contents = ContentsStorage;
     } else if (JustComputedResult) {
       continue;
+    } else if (OutKind == OutputKind::Dependencies) {
+      llvm::raw_svector_ostream OS(ContentsStorage);
+      if (auto E = CASDependencyCollector::replay(
+              Clang.getDependencyOutputOpts(), *CAS, BytesRef, OS))
+        llvm::report_fatal_error(std::move(E));
+      Contents = ContentsStorage;
     } else {
       Optional<llvm::cas::ObjectProxy> Bytes;
       if (Error E = CAS->getProxy(BytesID).moveInto(Bytes))

llvm/include/llvm/CAS/CASOutputBackend.h

@@ -36,6 +36,8 @@ class CASOutputBackend : public vfs::OutputBackend {
   /// the \p Kind string instead of its path.
   void addKindMap(StringRef Kind, StringRef Path);
 
+  CASDB &getCAS() const { return CAS; }
+
 private:
   Expected<std::unique_ptr<vfs::OutputFileImpl>>
   createFileImpl(StringRef Path, Optional<vfs::OutputConfig> Config) override;