[clang][deps] Preserve input ordering in the full output

This patch makes sure the ordering of TUs in the input CDB is preserved in the full output. Previously, the results were pushed into a global vector, potentially out-of-order and then sorted by the input file name. This is non-deterministic when the CDB contains multiple entries with the same input file. This patch fixes that by pre-allocating the output vector and writing directly to the position corresponding to the current input. This also eliminates one critical section.

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D145098
This commit is contained in:
Jan Svoboda 2023-03-01 11:31:49 -08:00
parent 8350494a61
commit 8640545008
3 changed files with 147 additions and 81 deletions

View File

@ -0,0 +1,64 @@
// This test checks that ordering of TUs in the input CDB is preserved in the full output.
// RUN: rm -rf %t
// RUN: split-file %s %t
//--- cdb.json.template
[
{
"directory": "DIR",
"file": "DIR/tu.c",
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.c -o DIR/tu1.o"
},
{
"directory": "DIR",
"file": "DIR/tu.c",
"command": "clang -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.c -o DIR/tu2.o"
}
]
//--- tu.c
// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
// CHECK: {
// CHECK-NEXT: "modules": [],
// CHECK-NEXT: "translation-units": [
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "{{.*}}",
// CHECK-NEXT: "clang-module-deps": [],
// CHECK-NEXT: "command-line": [
// CHECK: "-o",
// CHECK-NEXT: "[[PREFIX]]/tu1.o",
// CHECK: ],
// CHECK-NEXT: "executable": "clang",
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
// CHECK: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "{{.*}}",
// CHECK-NEXT: "clang-module-deps": [],
// CHECK-NEXT: "command-line": [
// CHECK: "-o",
// CHECK-NEXT: "[[PREFIX]]/tu2.o",
// CHECK: ],
// CHECK-NEXT: "executable": "clang",
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
// CHECK: ]
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: }

View File

@ -84,75 +84,6 @@
// CHECK-NEXT: { // CHECK-NEXT: {
// CHECK-NEXT: "commands": [ // CHECK-NEXT: "commands": [
// CHECK-NEXT: { // CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: {
// CHECK-NEXT: "context-hash": "[[HASH_H1]]",
// CHECK-NEXT: "module-name": "header1"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK-NOT: "-fimplicit-modules"
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}clang{{.*}}"
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: {
// CHECK-NEXT: "context-hash": "[[HASH_H1]]",
// CHECK-NEXT: "module-name": "header1"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK-NOT: "-fimplicit-modules"
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}clang{{.*}}"
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: {
// CHECK-NEXT: "context-hash": "[[HASH_H1]]",
// CHECK-NEXT: "module-name": "header1"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK-NOT: "-fimplicit-modules"
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}clang{{.*}}"
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU_DINCLUDE:[A-Z0-9]+]]", // CHECK-NEXT: "clang-context-hash": "[[HASH_TU_DINCLUDE:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [ // CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: { // CHECK-NEXT: {
@ -172,6 +103,75 @@
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input2.cpp" // CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input2.cpp"
// CHECK-NEXT: } // CHECK-NEXT: }
// CHECK-NEXT: ] // CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: {
// CHECK-NEXT: "context-hash": "[[HASH_H1]]",
// CHECK-NEXT: "module-name": "header1"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK-NOT: "-fimplicit-modules"
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}clang{{.*}}"
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: {
// CHECK-NEXT: "context-hash": "[[HASH_H1]]",
// CHECK-NEXT: "module-name": "header1"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK-NOT: "-fimplicit-modules"
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}clang{{.*}}"
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "commands": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
// CHECK-NEXT: "clang-module-deps": [
// CHECK-NEXT: {
// CHECK-NEXT: "context-hash": "[[HASH_H1]]",
// CHECK-NEXT: "module-name": "header1"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "command-line": [
// CHECK-NOT: "-fimplicit-modules"
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}clang{{.*}}"
// CHECK-NEXT: "file-deps": [
// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: } // CHECK-NEXT: }
// CHECK-NEXT: ] // CHECK-NEXT: ]
// CHECK-NEXT: } // CHECK-NEXT: }

View File

@ -274,6 +274,8 @@ static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) {
// Thread safe. // Thread safe.
class FullDeps { class FullDeps {
public: public:
FullDeps(size_t NumInputs) : Inputs(NumInputs) {}
void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps, void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
size_t InputIndex) { size_t InputIndex) {
mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex); mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex);
@ -286,8 +288,9 @@ public:
ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine); ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine);
ID.Commands = std::move(TUDeps.Commands); ID.Commands = std::move(TUDeps.Commands);
std::unique_lock<std::mutex> ul(Lock); assert(InputIndex < Inputs.size() && "Input index out of bounds");
Inputs.push_back(std::move(ID)); assert(Inputs[InputIndex].FileName.empty() && "Result already populated");
Inputs[InputIndex] = std::move(ID);
} }
void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) { void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
@ -344,10 +347,6 @@ public:
std::tie(B.ID.ModuleName, B.InputIndex); std::tie(B.ID.ModuleName, B.InputIndex);
}); });
llvm::sort(Inputs, [](const InputDeps &A, const InputDeps &B) {
return A.FileName < B.FileName;
});
using namespace llvm::json; using namespace llvm::json;
Array OutModules; Array OutModules;
@ -782,11 +781,14 @@ int main(int argc, const char **argv) {
AdjustingCompilations->getAllCompileCommands(); AdjustingCompilations->getAllCompileCommands();
std::atomic<bool> HadErrors(false); std::atomic<bool> HadErrors(false);
FullDeps FD; std::optional<FullDeps> FD;
P1689Deps PD; P1689Deps PD;
std::mutex Lock; std::mutex Lock;
size_t Index = 0; size_t Index = 0;
if (Format == ScanningOutputFormat::Full)
FD.emplace(ModuleName.empty() ? Inputs.size() : 0);
if (Verbose) { if (Verbose) {
llvm::outs() << "Running clang-scan-deps on " << Inputs.size() llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
<< " files using " << Pool.getThreadCount() << " workers\n"; << " files using " << Pool.getThreadCount() << " workers\n";
@ -875,14 +877,14 @@ int main(int argc, const char **argv) {
auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies( auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies(
*MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules, *MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules,
LookupOutput); LookupOutput);
if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, FD, if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, *FD,
LocalIndex, DependencyOS, Errs)) LocalIndex, DependencyOS, Errs))
HadErrors = true; HadErrors = true;
} else { } else {
auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies( auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies(
Input->CommandLine, CWD, AlreadySeenModules, LookupOutput); Input->CommandLine, CWD, AlreadySeenModules, LookupOutput);
if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex, if (handleTranslationUnitResult(Filename, MaybeTUDeps, *FD,
DependencyOS, Errs)) LocalIndex, DependencyOS, Errs))
HadErrors = true; HadErrors = true;
} }
} }
@ -891,11 +893,11 @@ int main(int argc, const char **argv) {
Pool.wait(); Pool.wait();
if (RoundTripArgs) if (RoundTripArgs)
if (FD.roundTripCommands(llvm::errs())) if (FD && FD->roundTripCommands(llvm::errs()))
HadErrors = true; HadErrors = true;
if (Format == ScanningOutputFormat::Full) if (Format == ScanningOutputFormat::Full)
FD.printFullOutput(llvm::outs()); FD->printFullOutput(llvm::outs());
else if (Format == ScanningOutputFormat::P1689) else if (Format == ScanningOutputFormat::P1689)
PD.printDependencies(llvm::outs()); PD.printDependencies(llvm::outs());