jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, benlangmuir.
Herald added subscribers: ributzka, mgrang.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 instead of
appending, writes to the index corresponding to the current input. This also
eliminates one critical section.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145098
Files:
clang/test/ClangScanDeps/modules-full-output-tu-order.c
clang/test/ClangScanDeps/modules-full.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -274,6 +274,8 @@
// Thread safe.
class FullDeps {
public:
+ void resize(size_t NumInputs) { Inputs.resize(NumInputs); }
+
void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
size_t InputIndex) {
mergeDeps(std::move(TUDeps.ModuleGraph), InputIndex);
@@ -286,8 +288,7 @@
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));
+ Inputs[InputIndex] = std::move(ID);
}
void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
@@ -344,10 +345,6 @@
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;
@@ -787,6 +784,9 @@
std::mutex Lock;
size_t Index = 0;
+ if (Format == ScanningOutputFormat::Full && ModuleName.empty())
+ FD.resize(Inputs.size());
+
if (Verbose) {
llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
<< " files using " << Pool.getThreadCount() << " workers\n";
Index: clang/test/ClangScanDeps/modules-full.cpp
===================================================================
--- clang/test/ClangScanDeps/modules-full.cpp
+++ 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: }
Index: clang/test/ClangScanDeps/modules-full-output-tu-order.c
===================================================================
--- /dev/null
+++ 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: }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
