Make the compilation cache key and the CAS outputs data output path independent

akyrtzi · akyrtzi · commit 4ceab205ff67 · 2022-08-10T15:59:27.000-07:00
Instead of storing CAS outputs by the specific path that was used at the time of invocation, associate them with an "output kind name" so that they are re-usable even with different output paths.

The practical benefit of this is that we can get cache hits when source files are part of multiple products (e.g. when source files are included in multiple Xcode test targets) or when PCH files end up with the same caching key and output but the build system is not in a position identify this.
diff --git a/clang/lib/Frontend/CompileJobCacheKey.cpp b/clang/lib/Frontend/CompileJobCacheKey.cpp
@@ -56,18 +56,29 @@ clang::createCompileJobCacheKey(CASDB &CAS, ArrayRef<const char *> CC1Args,
 
 Optional<llvm::cas::CASID>
 clang::createCompileJobCacheKey(CASDB &CAS, DiagnosticsEngine &Diags,
-                                const CompilerInvocation &Invocation) {
+                                const CompilerInvocation &OriginalInvocation) {
+  CompilerInvocation InvocationForCacheKey(OriginalInvocation);
+  FrontendOptions &FrontendOpts = InvocationForCacheKey.getFrontendOpts();
+  DiagnosticOptions &DiagOpts = InvocationForCacheKey.getDiagnosticOpts();
+  // Keep the key independent of the paths of these outputs.
+  FrontendOpts.OutputFile = "-";
+  InvocationForCacheKey.getDependencyOutputOpts().OutputFile = "-";
+  // We always generate the serialized diagnostics so the key is independent of
+  // the presence of '--serialize-diagnostics'.
+  DiagOpts.DiagnosticSerializationFile.clear();
+
   // Generate a new command-line in case Invocation has been canonicalized.
   llvm::BumpPtrAllocator Alloc;
   llvm::StringSaver Saver(Alloc);
   llvm::SmallVector<const char *> Argv;
   Argv.push_back("-cc1");
-  Invocation.generateCC1CommandLine(
+  InvocationForCacheKey.generateCC1CommandLine(
       Argv, [&Saver](const llvm::Twine &T) { return Saver.save(T).data(); });
 
   // FIXME: currently correct since the main executable is always in the root
   // from scanning, but we should probably make it explicit here...
-  StringRef RootIDString = Invocation.getFileSystemOpts().CASFileSystemRootID;
+  StringRef RootIDString =
+      InvocationForCacheKey.getFileSystemOpts().CASFileSystemRootID;
   Expected<llvm::cas::CASID> RootID = CAS.parseID(RootIDString);
   if (!RootID) {
     llvm::consumeError(RootID.takeError());
diff --git a/clang/test/CAS/fcache-compile-job-dependency-file.c b/clang/test/CAS/fcache-compile-job-dependency-file.c
@@ -1,23 +1,33 @@
 // RUN: rm -rf %t && mkdir -p %t
-// RUN: llvm-cas --cas %t/cas --ingest --data %s > %t/casid
+// RUN: split-file %s %t
+// RUN: llvm-cas --cas %t/cas --ingest --data %t > %t/casid
 //
 // RUN: %clang -cc1 -triple x86_64-apple-macos11 \
 // RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
-// RUN:   -Rcompile-job-cache -emit-obj -o %t/output.o \
-// RUN:   -dependency-file %t/deps.d -MT %t/output.o 2>&1 \
+// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o \
+// 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
+
 // RUN: ls %t/output.o && rm %t/output.o
-// RUN: ls %t/deps.d && mv %t/deps.d %t/deps.d.orig
 //
 // RUN: %clang -cc1 -triple x86_64-apple-macos11 \
 // RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
-// RUN:   -Rcompile-job-cache -emit-obj -o %t/output.o \
-// RUN:   -dependency-file %t/deps.d -MT %t/output.o 2>&1 \
+// RUN:   -Rcompile-job-cache %t/main.c -emit-obj -o %t/output.o \
+// RUN:   -dependency-file %t/deps2.d -MT depends 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CACHE-HIT
 //
 // RUN: ls %t/output.o
-// RUN: diff -u %t/deps.d %t/deps.d.orig
+// RUN: diff -u %t/deps1.d %t/deps2.d
 //
 // CACHE-HIT: remark: compile job cache hit
 // CACHE-MISS-NOT: remark: compile job cache hit
+
+//--- main.c
+#include "my_header.h"
+
+//--- my_header.h
diff --git a/clang/test/CAS/fcache-compile-job.c b/clang/test/CAS/fcache-compile-job.c
@@ -15,12 +15,7 @@
 // RUN: %clang -cc1 -triple x86_64-apple-macos11 \
 // RUN:   -fcas-path %t/cas -fcas-fs @%t/casid -fcache-compile-job \
 // RUN:   -Rcompile-job-cache-hit -emit-obj -o output.o 2>&1 \
-// RUN:   | FileCheck %s --allow-empty --check-prefix=CACHE-MISS
-// 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-hit -emit-obj -o output.o 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CACHE-HIT
+// RUN:   | FileCheck %s --allow-empty --check-prefix=CACHE-HIT
 // RUN: ls %t/output.o
 //
 // Check for a cache hit if the CAS moves:
diff --git a/clang/test/CAS/path-independent-cas-outputs.c b/clang/test/CAS/path-independent-cas-outputs.c
@@ -0,0 +1,74 @@
+// RUN: rm -rf %t && mkdir -p %t/a %t/b
+
+// Check that we got a cache hit even though the output paths are different.
+
+// RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/a/t1.o -MMD -MT dependencies -MF %t/a/t1.d --serialize-diagnostics %t/a/t1.dia -Rcompile-job-cache \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CACHE-MISS
+// RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/b/t2.o -MMD -MT dependencies -MF %t/b/t2.d --serialize-diagnostics %t/b/t2.dia -Rcompile-job-cache \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CACHE-HIT
+
+// Check PCH output
+
+// RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -x c-header %s -o %t/a/t1.pch -Rcompile-job-cache 2>&1 | FileCheck %s --check-prefix=CACHE-MISS
+// RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -x c-header %s -o %t/b/t2.pch -Rcompile-job-cache 2>&1 | FileCheck %s --check-prefix=CACHE-HIT
+
+// CACHE-MISS: remark: compile job cache miss
+// CACHE-HIT: remark: compile job cache hit
+
+// Repeat to diff outputs produced from each invocation. CAS path is different to avoid cache hits.
+
+// RUN: rm -rf %t && mkdir -p %t
+
+// Baseline to check we got expected outputs.
+// RUN: %clang -target x86_64-apple-macos11 -c %s -o %t/t.o -MMD -MT dependencies -MF %t/t.d --serialize-diagnostics %t/t.dia
+// RUN: env LLVM_CACHE_CAS_PATH=%t/a/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/a/t1.o -MMD -MT dependencies -MF %t/a/t1.d --serialize-diagnostics %t/a/t1.dia
+// RUN: env LLVM_CACHE_CAS_PATH=%t/b/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/b/t2.o -MMD -MT dependencies -MF %t/b/t2.d --serialize-diagnostics %t/b/t2.dia
+
+// RUN: diff %t/a/t1.o %t/b/t2.o
+// RUN: diff %t/t.o %t/a/t1.o
+
+// RUN: diff %t/a/t1.dia %t/b/t2.dia
+// RUN: diff %t/t.dia %t/a/t1.dia
+
+// RUN: diff %t/a/t1.d %t/b/t2.d
+// RUN: diff %t/t.d %t/a/t1.d
+
+// Baseline to check we got expected output.
+// RUN: %clang -target x86_64-apple-macos11 -x c-header %s -o %t/t.pch -Xclang -fno-pch-timestamp
+// RUN: env LLVM_CACHE_CAS_PATH=%t/a/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -x c-header %s -o %t/a/t1.pch
+// RUN: env LLVM_CACHE_CAS_PATH=%t/b/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -x c-header %s -o %t/b/t2.pch
+
+// RUN: diff %t/a/t1.pch %t/b/t2.pch
+// RUN: diff %t/t.pch %t/a/t1.pch
+
+// Check that caching is independent of whether '--serialize-diagnostics' exists or not.
+
+// Check with the option missing then present.
+// RUN: env LLVM_CACHE_CAS_PATH=%t/d1/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t1.o -Rcompile-job-cache \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CACHE-MISS
+// RUN: env LLVM_CACHE_CAS_PATH=%t/d1/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t2.o --serialize-diagnostics %t/t1.dia -Rcompile-job-cache \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CACHE-HIT
+
+// Check with the option present then missing.
+// RUN: env LLVM_CACHE_CAS_PATH=%t/d2/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t1.o --serialize-diagnostics %t/t2.dia -Rcompile-job-cache \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CACHE-MISS
+// RUN: env LLVM_CACHE_CAS_PATH=%t/d2/cas %clang-cache \
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t2.o -Rcompile-job-cache \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CACHE-HIT
+
+// RUN: diff %t/t1.dia %t/t2.dia
+// RUN: diff %t/t.dia %t/t1.dia
+
+#warning some warning
+void test() {}
diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
@@ -236,6 +236,27 @@ namespace {
 //     handled.
 class CompileJobCache {
 public:
+  /// Categorization for the output kinds that is used to decouple the
+  /// compilation cache key from the specific output paths.
+  enum class OutputKind {
+    MainOutput,
+    SerializedDiagnostics,
+    Dependencies,
+  };
+  static ArrayRef<OutputKind> getAllOutputKinds() {
+    static const OutputKind AllOutputKinds[] = {
+        OutputKind::MainOutput, OutputKind::SerializedDiagnostics,
+        OutputKind::Dependencies};
+    return llvm::makeArrayRef(AllOutputKinds);
+  }
+
+  static StringRef getOutputKindName(OutputKind Kind);
+
+  /// \returns \p None if \p Name doesn't match one of the output kind names.
+  static Optional<OutputKind> getOutputKindForName(StringRef Name);
+
+  StringRef getPathForOutputKind(OutputKind Kind);
+
   /// Canonicalize \p Clang.
   ///
   /// Return status if should exit immediately, otherwise None.
@@ -267,19 +288,60 @@ class CompileJobCache {
   SmallString<256> ResultDiags;
   Optional<llvm::cas::CASID> ResultCacheKey;
   std::unique_ptr<llvm::raw_ostream> ResultDiagsOS;
+  SmallString<256> SerialDiagsBuf;
   IntrusiveRefCntPtr<llvm::cas::CASOutputBackend> CASOutputs;
   std::string OutputFile;
+  std::string SerialDiagsFile;
+  std::string DependenciesFile;
   Optional<llvm::vfs::OutputFile> SerialDiagsOutput;
 };
 } // end anonymous namespace
 
+static constexpr llvm::StringLiteral MainOutputKindName = "<output>";
+static constexpr llvm::StringLiteral SerializedDiagnosticsKindName =
+    "<serial-diags>";
+static constexpr llvm::StringLiteral DependenciesOutputKindName =
+    "<dependencies>";
+
+StringRef CompileJobCache::getOutputKindName(OutputKind Kind) {
+  switch (Kind) {
+  case OutputKind::MainOutput:
+    return MainOutputKindName;
+  case OutputKind::SerializedDiagnostics:
+    return SerializedDiagnosticsKindName;
+  case OutputKind::Dependencies:
+    return DependenciesOutputKindName;
+  }
+}
+
+Optional<CompileJobCache::OutputKind>
+CompileJobCache::getOutputKindForName(StringRef Name) {
+  return llvm::StringSwitch<Optional<OutputKind>>(Name)
+      .Case(MainOutputKindName, OutputKind::MainOutput)
+      .Case(SerializedDiagnosticsKindName, OutputKind::SerializedDiagnostics)
+      .Case(DependenciesOutputKindName, OutputKind::Dependencies)
+      .Default(None);
+}
+
+StringRef CompileJobCache::getPathForOutputKind(OutputKind Kind) {
+  switch (Kind) {
+  case OutputKind::MainOutput:
+    return OutputFile;
+  case OutputKind::SerializedDiagnostics:
+    return SerialDiagsFile;
+  case OutputKind::Dependencies:
+    return DependenciesFile;
+  }
+}
+
 Optional<int> CompileJobCache::initialize(CompilerInstance &Clang) {
   CompilerInvocation &Invocation = Clang.getInvocation();
   DiagnosticsEngine &Diags = Clang.getDiagnostics();
+  FrontendOptions &FrontendOpts = Invocation.getFrontendOpts();
 
   // Extract whether caching is on (and canonicalize setting).
-  CacheCompileJob = Invocation.getFrontendOpts().CacheCompileJob;
-  Invocation.getFrontendOpts().CacheCompileJob = false;
+  CacheCompileJob = FrontendOpts.CacheCompileJob;
+  FrontendOpts.CacheCompileJob = false;
 
   // Nothing else to do if we're not caching.
   if (!CacheCompileJob)
@@ -300,10 +362,11 @@ Optional<int> CompileJobCache::initialize(CompilerInstance &Clang) {
   // TODO: Canonicalize DiagnosticOptions here to be "serialized" only. Pass in
   // a hook to mirror diagnostics to stderr (when writing there), and handle
   // other outputs during replay.
+  FrontendOpts.IncludeTimestamps = false;
 
-  // TODO: Canonicalize OutputFile to "-" here. During replay, move it and
-  // derived outputs to the right place.
-  OutputFile = Invocation.getFrontendOpts().OutputFile;
+  OutputFile = FrontendOpts.OutputFile;
+  SerialDiagsFile = Invocation.getDiagnosticOpts().DiagnosticSerializationFile;
+  DependenciesFile = Invocation.getDependencyOutputOpts().OutputFile;
   return None;
 }
 
@@ -387,6 +450,11 @@ 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);
+  for (OutputKind K : getAllOutputKinds()) {
+    StringRef OutPath = getPathForOutputKind(K);
+    if (!OutPath.empty())
+      CASOutputs->addKindMap(getOutputKindName(K), OutPath);
+  }
 
   Clang.setOutputBackend(llvm::vfs::makeMirroringOutputBackend(
       CASOutputs, std::move(OnDiskOutputs)));
@@ -442,6 +510,14 @@ Optional<int> CompileJobCache::tryReplayCachedResult(CompilerInstance &Clang) {
         OutputFile, &DiagOpts, /*MergeChildRecords*/ false, std::move(*OS));
     Diags.setClient(new ChainedDiagnosticConsumer(
         Diags.takeClient(), std::move(SerializedConsumer)));
+  } else {
+    // We always generate the serialized diagnostics so the key is independent
+    // of the presence of '--serialize-diagnostics'.
+    auto OS = std::make_unique<llvm::raw_svector_ostream>(SerialDiagsBuf);
+    auto SerializedConsumer = clang::serialized_diags::create(
+        StringRef(), &DiagOpts, /*MergeChildRecords*/ false, std::move(OS));
+    Diags.setClient(new ChainedDiagnosticConsumer(
+        Diags.takeClient(), std::move(SerializedConsumer)));
   }
 
   return None;
@@ -476,6 +552,21 @@ void CompileJobCache::finishComputedResult(CompilerInstance &Clang,
     return;
 
   // FIXME: Stop calling report_fatal_error().
+  if (!SerialDiagsOutput) {
+    // Not requested to get a serialized diagnostics file but we generated it
+    // and will store it regardless so that the key is independent of the
+    // presence of '--serialize-diagnostics'.
+    Expected<llvm::cas::ObjectProxy> SerialDiags =
+        CAS->createProxy(None, SerialDiagsBuf);
+    // FIXME: Stop calling report_fatal_error().
+    if (!SerialDiags)
+      llvm::report_fatal_error(SerialDiags.takeError());
+    if (Error E = CASOutputs->addObject(
+            getOutputKindName(OutputKind::SerializedDiagnostics),
+            SerialDiags->getRef()))
+      llvm::report_fatal_error(std::move(E));
+  }
+
   Expected<llvm::cas::ObjectProxy> Outputs = CASOutputs->getCASProxy();
   if (!Outputs)
     llvm::report_fatal_error(Outputs.takeError());
@@ -557,20 +648,28 @@ Optional<int> CompileJobCache::replayCachedResult(CompilerInstance &Clang,
     llvm::cas::CASID PathID = Outputs->getReferenceID(I);
     llvm::cas::CASID BytesID = Outputs->getReferenceID(I + 1);
 
-    Optional<llvm::cas::ObjectProxy> Path;
-    if (Error E = CAS->getProxy(PathID).moveInto(Path))
+    Optional<llvm::cas::ObjectProxy> PathProxy;
+    if (Error E = CAS->getProxy(PathID).moveInto(PathProxy))
       llvm::report_fatal_error(std::move(E));
 
+    Optional<OutputKind> OutKind = getOutputKindForName(PathProxy->getData());
+    StringRef Path =
+        OutKind ? getPathForOutputKind(*OutKind) : PathProxy->getData();
+    if (Path.empty()) {
+      // The output may be always generated but not needed with this invocation,
+      // like the serialized diagnostics file.
+      continue;
+    }
+
     Optional<StringRef> Contents;
     SmallString<50> ContentsStorage;
     Optional<llvm::cas::ObjectProxy> Bytes;
     if (Error E = CAS->getProxy(BytesID).moveInto(Bytes))
       llvm::report_fatal_error(std::move(E));
     Contents = Bytes->getData();
     std::unique_ptr<llvm::FileOutputBuffer> Output;
-    if (Error E =
-            llvm::FileOutputBuffer::create(Path->getData(), Contents->size())
-                .moveInto(Output))
+    if (Error E = llvm::FileOutputBuffer::create(Path, Contents->size())
+                      .moveInto(Output))
       llvm::report_fatal_error(std::move(E));
     llvm::copy(*Contents, Output->getBufferStart());
     if (llvm::Error E = Output->commit())
diff --git a/llvm/include/llvm/CAS/CASOutputBackend.h b/llvm/include/llvm/CAS/CASOutputBackend.h
@@ -27,8 +27,14 @@ class CASOutputBackend : public vfs::OutputBackend {
   /// Create a top-level tree for all created files. This will contain all files
   Expected<ObjectProxy> getCASProxy();
 
-  /// Add a CAS object to the path in the output backend.
-  Error addObject(StringRef Path, ObjectRef Object);
+  /// Add a CAS object in the output backend associated with the given name,
+  /// which can be a path or a "kind" string.
+  Error addObject(StringRef Name, ObjectRef Object);
+
+  /// Add an association of a "kind" string with a particular output path.
+  /// When the output for \p Path is encountered it will be associated with
+  /// the \p Kind string instead of its path.
+  void addKindMap(StringRef Kind, StringRef Path);
 
 private:
   Expected<std::unique_ptr<vfs::OutputFileImpl>>
diff --git a/llvm/lib/CAS/CASOutputBackend.cpp b/llvm/lib/CAS/CASOutputBackend.cpp
