Skip to content

[stable][clang][cas] Ignore the -fdepfile-entry= dependency option when calculating the cache key #5282

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
4 changes: 2 additions & 2 deletions clang/include/clang/Frontend/CASDependencyCollector.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class CASDependencyCollector : public DependencyFileGenerator {
/// \param Callback Callback that receives the resulting dependencies on
/// completion, or \c None if an error occurred.
CASDependencyCollector(
const DependencyOutputOptions &Opts, cas::ObjectStore &CAS,
DependencyOutputOptions Opts, cas::ObjectStore &CAS,
std::function<void(Optional<cas::ObjectRef>)> Callback);

/// Replay the given result, which should have been created by a
Expand All @@ -51,4 +51,4 @@ class CASDependencyCollector : public DependencyFileGenerator {

} // namespace clang

#endif // LLVM_CLANG_FRONTEND_CASDEPENDENCYCOLLECTOR_H
#endif // LLVM_CLANG_FRONTEND_CASDEPENDENCYCOLLECTOR_H
22 changes: 20 additions & 2 deletions clang/lib/Frontend/CASDependencyCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,19 @@
using namespace clang;
using namespace clang::cas;

/// \returns \p DependencyOutputOptions but with \p ExtraDeps cleared out.
///
/// This is useful to avoid recording these filenames in the CAS.
static DependencyOutputOptions dropExtraDeps(DependencyOutputOptions Opts) {
Opts.ExtraDeps.clear();
return Opts;
}

CASDependencyCollector::CASDependencyCollector(
const DependencyOutputOptions &Opts, cas::ObjectStore &CAS,
DependencyOutputOptions Opts, cas::ObjectStore &CAS,
std::function<void(Optional<cas::ObjectRef>)> Callback)
: DependencyFileGenerator(Opts, llvm::vfs::makeNullOutputBackend()),
: DependencyFileGenerator(dropExtraDeps(std::move(Opts)),
llvm::vfs::makeNullOutputBackend()),
CAS(CAS), Callback(std::move(Callback)) {}

llvm::Error CASDependencyCollector::replay(const DependencyOutputOptions &Opts,
Expand All @@ -29,6 +38,15 @@ llvm::Error CASDependencyCollector::replay(const DependencyOutputOptions &Opts,
return Refs.takeError();

CASDependencyCollector DC(Opts, CAS, nullptr);

// Add the filenames from DependencyOutputOptions::ExtraDeps. These are kept
// out of the compilation cache key since they can be passed-in and added at
// the point where the dependency file is generated, without needing to affect
// the cached compilation.
for (const std::pair<std::string, ExtraDepKind> &Dep : Opts.ExtraDeps) {
DC.addDependency(Dep.first);
}

auto Err = Refs->forEachReference([&](ObjectRef Ref) -> llvm::Error {
auto PathHandle = CAS.getProxy(Ref);
if (!PathHandle)
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Frontend/CompileJobCacheKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ clang::createCompileJobCacheKey(ObjectStore &CAS, DiagnosticsEngine &Diags,
if (!DepOpts.Targets.empty())
DepOpts.Targets = {"-"};
DepOpts.UsePhonyTargets = false;
// These are added in when the dependency file is generated, but they don't
// affect the actual compilation.
DepOpts.ExtraDeps.clear();

// Generate a new command-line in case Invocation has been canonicalized.
llvm::BumpPtrAllocator Alloc;
Expand Down
26 changes: 25 additions & 1 deletion clang/test/CAS/fcache-compile-job-dependency-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@
// RUN: %clang -cc1 -triple x86_64-apple-macos11 \
// RUN: -fcas-path %t/cas -faction-cache-path %t/cache -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: -dependency-file %t/deps3.d -MT other1 -MT other2 -MP -fdepfile-entry=extra-depfile.json 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: extra-depfile.json
// DEPS_OTHER: main.c
// DEPS_OTHER: my_header.h
// DEPS_OTHER-NOT: sys.h
Expand All @@ -56,6 +57,29 @@
// DEPS_SYS: my_header.h
// DEPS_SYS: sys.h

// Using another cache path to avoid reusing artifacts.

// RUN: %clang -cc1 -triple x86_64-apple-macos11 \
// RUN: -fcas-path %t/cas -faction-cache-path %t/cache2 -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/deps-depfile1.d -MT deps -fdepfile-entry=extra-depfile.json -fdepfile-entry=%t/main.c 2>&1 \
// RUN: | FileCheck %s --check-prefix=CACHE-MISS

// RUN: FileCheck %s --input-file=%t/deps-depfile1.d --check-prefix=DEPS_DEPFILE1
// DEPS_DEPFILE1: deps:
// DEPS_DEPFILE1: extra-depfile.json
// DEPS_DEPFILE1: main.c

// RUN: %clang -cc1 -triple x86_64-apple-macos11 \
// RUN: -fcas-path %t/cas -faction-cache-path %t/cache2 -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/deps-depfile2.d -MT deps 2>&1 \
// RUN: | FileCheck %s --check-prefix=CACHE-HIT

// RUN: FileCheck %s --input-file=%t/deps-depfile2.d --check-prefix=DEPS_DEPFILE2
// DEPS_DEPFILE2-NOT: extra-depfile.json
// DEPS_DEPFILE2: main.c

//--- main.c
#include "my_header.h"
#include <sys.h>
Expand Down