Author: Jan Svoboda Date: 2023-03-02T13:42:11-08:00 New Revision: 864054500830d708cc9f7d8a53195a95488e4e0b
URL: https://github.com/llvm/llvm-project/commit/864054500830d708cc9f7d8a53195a95488e4e0b DIFF: https://github.com/llvm/llvm-project/commit/864054500830d708cc9f7d8a53195a95488e4e0b.diff LOG: [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 Added: clang/test/ClangScanDeps/modules-full-output-tu-order.c Modified: clang/test/ClangScanDeps/modules-full.cpp clang/tools/clang-scan-deps/ClangScanDeps.cpp Removed: ################################################################################ diff --git a/clang/test/ClangScanDeps/modules-full-output-tu-order.c b/clang/test/ClangScanDeps/modules-full-output-tu-order.c new file mode 100644 index 0000000000000..dd5002809daa9 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-full-output-tu-order.c @@ -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: } diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp index a19aafddcc140..59efef0ecbaa6 100644 --- a/clang/test/ClangScanDeps/modules-full.cpp +++ b/clang/test/ClangScanDeps/modules-full.cpp @@ -84,23 +84,23 @@ // CHECK-NEXT: { // CHECK-NEXT: "commands": [ // CHECK-NEXT: { -// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]", +// CHECK-NEXT: "clang-context-hash": "[[HASH_TU_DINCLUDE:[A-Z0-9]+]]", // CHECK-NEXT: "clang-module-deps": [ // CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_H1]]", +// CHECK-NEXT: "context-hash": "[[HASH_H1_DINCLUDE]]", // 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: "-fmodule-file={{.*}}[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1_DINCLUDE]]/header1-{{[A-Z0-9]+}}.pcm" // CHECK: ], // CHECK-NEXT: "executable": "{{.*}}clang{{.*}}" // CHECK-NEXT: "file-deps": [ -// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp" +// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input2.cpp" // CHECK-NEXT: ], -// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp" +// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input2.cpp" // CHECK-NEXT: } // CHECK-NEXT: ] // CHECK-NEXT: }, @@ -153,23 +153,23 @@ // 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:[A-Z0-9]+]]", // CHECK-NEXT: "clang-module-deps": [ // CHECK-NEXT: { -// CHECK-NEXT: "context-hash": "[[HASH_H1_DINCLUDE]]", +// 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_DINCLUDE]]/header1-{{[A-Z0-9]+}}.pcm" +// 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_input2.cpp" +// CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp" // CHECK-NEXT: ], -// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input2.cpp" +// CHECK-NEXT: "input-file": "[[PREFIX]]/modules_cdb_input.cpp" // CHECK-NEXT: } // CHECK-NEXT: ] // CHECK-NEXT: } diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 2cca188e28bb6..f200c8d6d8b71 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -274,6 +274,8 @@ static llvm::json::Array toJSONSorted(std::vector<ModuleID> V) { // Thread safe. class FullDeps { public: + FullDeps(size_t NumInputs) : Inputs(NumInputs) {} + void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps, size_t InputIndex) { mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex); @@ -286,8 +288,9 @@ class FullDeps { ID.DriverCommandLine = std::move(TUDeps.DriverCommandLine); ID.Commands = std::move(TUDeps.Commands); - std::unique_lock<std::mutex> ul(Lock); - Inputs.push_back(std::move(ID)); + assert(InputIndex < Inputs.size() && "Input index out of bounds"); + assert(Inputs[InputIndex].FileName.empty() && "Result already populated"); + Inputs[InputIndex] = std::move(ID); } void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) { @@ -344,10 +347,6 @@ class FullDeps { 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; Array OutModules; @@ -782,11 +781,14 @@ int main(int argc, const char **argv) { AdjustingCompilations->getAllCompileCommands(); std::atomic<bool> HadErrors(false); - FullDeps FD; + std::optional<FullDeps> FD; P1689Deps PD; std::mutex Lock; size_t Index = 0; + if (Format == ScanningOutputFormat::Full) + FD.emplace(ModuleName.empty() ? Inputs.size() : 0); + if (Verbose) { llvm::outs() << "Running clang-scan-deps on " << Inputs.size() << " files using " << Pool.getThreadCount() << " workers\n"; @@ -875,14 +877,14 @@ int main(int argc, const char **argv) { auto MaybeModuleDepsGraph = WorkerTools[I]->getModuleDependencies( *MaybeModuleName, Input->CommandLine, CWD, AlreadySeenModules, LookupOutput); - if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, FD, + if (handleModuleResult(*MaybeModuleName, MaybeModuleDepsGraph, *FD, LocalIndex, DependencyOS, Errs)) HadErrors = true; } else { auto MaybeTUDeps = WorkerTools[I]->getTranslationUnitDependencies( Input->CommandLine, CWD, AlreadySeenModules, LookupOutput); - if (handleTranslationUnitResult(Filename, MaybeTUDeps, FD, LocalIndex, - DependencyOS, Errs)) + if (handleTranslationUnitResult(Filename, MaybeTUDeps, *FD, + LocalIndex, DependencyOS, Errs)) HadErrors = true; } } @@ -891,11 +893,11 @@ int main(int argc, const char **argv) { Pool.wait(); if (RoundTripArgs) - if (FD.roundTripCommands(llvm::errs())) + if (FD && FD->roundTripCommands(llvm::errs())) HadErrors = true; if (Format == ScanningOutputFormat::Full) - FD.printFullOutput(llvm::outs()); + FD->printFullOutput(llvm::outs()); else if (Format == ScanningOutputFormat::P1689) PD.printDependencies(llvm::outs()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
