[clang] 8640545 - [clang][deps] Preserve input ordering in the full output
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 2 13:42:16 PST 2023
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());
More information about the cfe-commits
mailing list