[DTLTO] [LLVM] Initial DTLTO cache implementation #156433

romanova-ekaterina · 2025-09-02T09:34:54Z

This patch implements DTLTO cache. DTLTO cache is implemented the same
way as ThinLTO cache. In fact the same class Cache is used for both of them.

Because parameters for codegen are different for DTLTO and ThinLTO
(DTLTO codegen is done by invoking clang and its codegen parameters are
not fully synchronized with codegen parameters used by LTO backend).
The object files generated by DTLTO and ThinLTO might be different and
shouldn't be mixed. If ThinLTO and DTLTO share the same cache
directory, the cache file won't interfere with each other.

I added a couple of test files in cross-project-test/dtlto directory,
but if more tests are required for initial implementation, I could add
them.

github-actions · 2025-09-02T09:37:49Z

✅ With the latest revision this PR passed the C/C++ code formatter.

This patch implements DTLTO cache. DTLTO cache is implemented the same way as ThinLTO cache. In fact the same class Cache is used for both of them. Because parameters for codegen are different for DTLTO and ThinLTO (DTLTO codegen is done by invoking clang and its codegen parameters are not fully synchronized with codegen parameters used by LTO backend). The object files generated by DTLTO and ThinLTO might be different and shouldn't be mixed. If ThinLTO and DTLTO share the same cache directory, the cache file won't interfere with each other. I added a couple of test files in cross-project-test/dtlto directory, but if more tests are required for initial implementation, I could add them.

llvmbot · 2025-09-04T06:14:03Z

@llvm/pr-subscribers-lto

Author: Katya Romanova (romanova-ekaterina)

Changes

This patch implements DTLTO cache. DTLTO cache is implemented the same
way as ThinLTO cache. In fact the same class Cache is used for both of them.

Because parameters for codegen are different for DTLTO and ThinLTO
(DTLTO codegen is done by invoking clang and its codegen parameters are
not fully synchronized with codegen parameters used by LTO backend).
The object files generated by DTLTO and ThinLTO might be different and
shouldn't be mixed. If ThinLTO and DTLTO share the same cache
directory, the cache file won't interfere with each other.

I added a couple of test files in cross-project-test/dtlto directory,
but if more tests are required for initial implementation, I could add
them.

Full diff: https://github.com/llvm/llvm-project/pull/156433.diff

4 Files Affected:

(added) cross-project-tests/dtlto/dtlto-cache.test (+89)
(added) cross-project-tests/dtlto/dtlto-thinlto-cache.test (+66)
(modified) llvm/include/llvm/LTO/Config.h (+1)
(modified) llvm/lib/LTO/LTO.cpp (+98-33)

diff --git a/cross-project-tests/dtlto/dtlto-cache.test b/cross-project-tests/dtlto/dtlto-cache.test
new file mode 100644
index 0000000000000..b98d4dbb433bb
--- /dev/null
+++ b/cross-project-tests/dtlto/dtlto-cache.test
@@ -0,0 +1,89 @@
+REQUIRES: x86-registered-target, ld.lld
+
+# Show that the ThinLTO cache works with DTLTO.
+
+RUN: rm -rf %t && split-file %s %t && cd %t
+
+# Compile source files into bitcode files.
+RUN: %clang -O2 --target=x86_64-linux-gnu -flto=thin -c foo.c main.c
+
+# Execute the linker and check that the cache is populated.
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.o foo.o -o populate1.elf \
+RUN:   -Wl,--thinlto-distributor=%python \
+RUN:   -Wl,--thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
+RUN:   -Wl,--thinlto-remote-compiler=%clang \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+# Check that there are two backend compilation jobs occurred.
+RUN: grep -wo args populate1.*.dist-file.json | wc -l | grep -qx 3
+RUN: ls cache.dir/llvmcache.timestamp
+RUN: ls cache.dir | count 3
+
+# Execute the linker again and check that a fully populated cache is used correctly, 
+# i.e., no additional cache entries are created for cache hits.
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.o foo.o -o populate2.elf \
+RUN:   -Wl,--thinlto-distributor=%python \
+RUN:   -Wl,--thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
+RUN:   -Wl,--thinlto-remote-compiler=%clang \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+# Check that there are no backend compilation jobs occurred.
+RUN: grep -wo args populate2.*.dist-file.json | wc -l | grep -qx 1
+RUN: ls cache.dir | count 3
+
+RUN: %clang -O0 --target=x86_64-linux-gnu -flto=thin -c foo.c -o foo.O0.o
+RUN: %clang -O0 --target=x86_64-linux-gnu -flto=thin -c main.c -o main.O0.o
+
+# Execute the linker again and check that the cache is populated correctly when there 
+# are no cache hits but there are existing cache entries.
+# As a side effect, this also verifies that the optimization level is considered when 
+# evaluating the cache entry key.
+
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.O0.o foo.O0.o -o populate3.elf \
+RUN:   -Wl,--thinlto-distributor=%python \
+RUN:   -Wl,--thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
+RUN:   -Wl,--thinlto-remote-compiler=%clang \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+# Check that there are two new backend compilation jobs occurred.
+RUN: grep -wo args populate3.*.dist-file.json | wc -l | grep -qx 3
+RUN: ls cache.dir | count 5
+
+RUN: %clang -O2 --target=x86_64-linux-gnu -flto=thin -c main-partial.c 
+
+# Execute the linker and check that everything works correctly with the partially populated cache;
+# One more cache entry should be generated after this run.
+
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main-partial.o foo.o -o main-partial.elf \
+RUN:   -Wl,--thinlto-distributor=%python \
+RUN:   -Wl,--thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
+RUN:   -Wl,--thinlto-remote-compiler=%clang \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+# Check that there is one new backend compilation jobs occurred.
+RUN: grep -wo args main-partial.*.dist-file.json | wc -l | grep -qx 2
+RUN: ls cache.dir | count 6
+
+#--- foo.c
+volatile int foo_int;
+__attribute__((retain)) int foo(int x) { return x + foo_int; }
+
+#--- main.c
+extern int foo(int x);
+__attribute__((retain)) int main(int argc, char** argv) {
+  return foo(argc);
+}
+
+#--- main-partial.c
+extern int foo(int x);
+__attribute__((retain)) int main(int argc, char** argv) {
+  return foo(argc+1);
+}
diff --git a/cross-project-tests/dtlto/dtlto-thinlto-cache.test b/cross-project-tests/dtlto/dtlto-thinlto-cache.test
new file mode 100644
index 0000000000000..d71e4aa5f131d
--- /dev/null
+++ b/cross-project-tests/dtlto/dtlto-thinlto-cache.test
@@ -0,0 +1,66 @@
+REQUIRES: x86-registered-target, ld.lld
+
+# This test verifies that a cache populated by a ThinLTO link is not reused by a DTLTO link and vice versa.
+
+RUN: rm -rf %t && split-file %s %t && cd %t
+
+# Compile source files into bitcode files.
+RUN: %clang -O2 --target=x86_64-linux-gnu -flto=thin -c foo.c main.c
+
+# Execute the linker and check that ThinLTO cache is populated.
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.o foo.o -o main.elf \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+RUN: ls cache.dir/llvmcache.timestamp
+RUN: ls cache.dir | count 3
+
+# Execute the linker and check that DTLTO adds additional entries to the ThinLTO cache, implying they do not share entries.
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.o foo.o -o populate1.elf \
+RUN:   -Wl,--thinlto-distributor=%python \
+RUN:   -Wl,--thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
+RUN:   -Wl,--thinlto-remote-compiler=%clang \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+# Check that there are two backend compilation jobs occurred.
+RUN: grep -wo args populate1.*.dist-file.json | wc -l | grep -qx 3
+RUN: ls cache.dir | count 5
+
+# Clean up cache directory.
+RUN: rm -rf cache.dir
+
+# Execute the linker and check that DTLTO cache is populated. 
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.o foo.o -o populate2.elf \
+RUN:   -Wl,--thinlto-distributor=%python \
+RUN:   -Wl,--thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \
+RUN:   -Wl,--thinlto-remote-compiler=%clang \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+# Check that there are two backend compilation jobs occurred.
+RUN: grep -wo args populate2.*.dist-file.json | wc -l | grep -qx 3
+RUN: ls cache.dir/llvmcache.timestamp
+RUN: ls cache.dir | count 3
+
+# Execute the linker and check that DTLTO adds additional entries to the ThinLTO cache, 
+# implying they do not share entries.
+RUN: %clang -O2 --target=x86_64-linux-gnu -Werror -flto=thin -fuse-ld=lld -nostdlib -e main \
+RUN:   main.o foo.o -o main.elf \
+RUN:   -Wl,--thinlto-cache-dir=cache.dir \
+RUN:   -Wl,--save-temps
+
+RUN: ls cache.dir | count 5
+
+#--- foo.c
+volatile int foo_int;
+__attribute__((retain)) int foo(int x) { return x + foo_int; }
+
+#--- main.c
+extern int foo(int x);
+__attribute__((retain)) int main(int argc, char** argv) {
+  return foo(argc);
+}
diff --git a/llvm/include/llvm/LTO/Config.h b/llvm/include/llvm/LTO/Config.h
index 50e143c518213..f5cd2e79e137c 100644
--- a/llvm/include/llvm/LTO/Config.h
+++ b/llvm/include/llvm/LTO/Config.h
@@ -281,6 +281,7 @@ struct Config {
   LLVM_ABI Error addSaveTemps(std::string OutputFileName,
                               bool UseInputModulePath = false,
                               const DenseSet<StringRef> &SaveTempsArgs = {});
+  mutable uint8_t Dtlto = 0;
 };
 
 struct LTOLLVMDiagnosticHandler : public DiagnosticHandler {
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 35d24c17bbd93..c086cffe9f9c9 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -168,6 +168,7 @@ std::string llvm::computeLTOCacheKey(
   AddString(Conf.OverrideTriple);
   AddString(Conf.DefaultTriple);
   AddString(Conf.DwoDir);
+  AddUint8(Conf.Dtlto);
 
   // Include the hash for the current module
   auto ModHash = Index.getModuleHash(ModuleID);
@@ -2244,7 +2245,7 @@ class OutOfProcessThinBackend : public CGThinBackend {
 
   SmallVector<StringRef, 0> CodegenOptions;
   DenseSet<StringRef> CommonInputs;
-
+  std::atomic<uint64_t> CachedJobs{0};
   // Information specific to individual backend compilation job.
   struct Job {
     unsigned Task;
@@ -2252,6 +2253,9 @@ class OutOfProcessThinBackend : public CGThinBackend {
     StringRef NativeObjectPath;
     StringRef SummaryIndexPath;
     ImportsFilesContainer ImportsFiles;
+    std::string CacheKey;
+    AddStreamFn CacheAddStream;
+    bool Cached = false;
   };
   // The set of backend compilations jobs.
   SmallVector<Job> Jobs;
@@ -2265,12 +2269,15 @@ class OutOfProcessThinBackend : public CGThinBackend {
   // The target triple to supply for backend compilations.
   llvm::Triple Triple;
 
+  // Cache
+  FileCache Cache;
+
 public:
   OutOfProcessThinBackend(
       const Config &Conf, ModuleSummaryIndex &CombinedIndex,
       ThreadPoolStrategy ThinLTOParallelism,
       const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
-      AddStreamFn AddStream, lto::IndexWriteCallback OnWrite,
+      AddStreamFn AddStream, FileCache CacheFn, lto::IndexWriteCallback OnWrite,
       bool ShouldEmitIndexFiles, bool ShouldEmitImportsFiles,
       StringRef LinkerOutputFile, StringRef Distributor,
       ArrayRef<StringRef> DistributorArgs, StringRef RemoteCompiler,
@@ -2280,7 +2287,8 @@ class OutOfProcessThinBackend : public CGThinBackend {
                       ShouldEmitImportsFiles, ThinLTOParallelism),
         LinkerOutputFile(LinkerOutputFile), DistributorPath(Distributor),
         DistributorArgs(DistributorArgs), RemoteCompiler(RemoteCompiler),
-        RemoteCompilerArgs(RemoteCompilerArgs), SaveTemps(SaveTemps) {}
+        RemoteCompilerArgs(RemoteCompilerArgs), SaveTemps(SaveTemps),
+        Cache(std::move(CacheFn)) {}
 
   virtual void setup(unsigned ThinLTONumTasks, unsigned ThinLTOTaskOffset,
                      llvm::Triple Triple) override {
@@ -2288,6 +2296,7 @@ class OutOfProcessThinBackend : public CGThinBackend {
     Jobs.resize((size_t)ThinLTONumTasks);
     this->ThinLTOTaskOffset = ThinLTOTaskOffset;
     this->Triple = Triple;
+    this->Conf.Dtlto = 1;
   }
 
   Error start(
@@ -2304,13 +2313,14 @@ class OutOfProcessThinBackend : public CGThinBackend {
                                        itostr(Task) + "." + UID + ".native.o");
 
     Job &J = Jobs[Task - ThinLTOTaskOffset];
-    J = {
-        Task,
-        ModulePath,
-        Saver.save(ObjFilePath.str()),
-        Saver.save(ObjFilePath.str() + ".thinlto.bc"),
-        {} // Filled in by emitFiles below.
-    };
+    J = {Task,
+         ModulePath,
+         Saver.save(ObjFilePath.str()),
+         Saver.save(ObjFilePath.str() + ".thinlto.bc"),
+         {}, // Filled in by emitFiles below.
+         "",
+         nullptr,
+         false};
 
     assert(ModuleToDefinedGVSummaries.count(ModulePath));
 
@@ -2326,6 +2336,35 @@ class OutOfProcessThinBackend : public CGThinBackend {
             else
               Err = std::move(E);
           }
+
+          if (Cache.isValid() &&
+              CombinedIndex.modulePaths().count(J.ModuleID) &&
+              all_of(CombinedIndex.getModuleHash(J.ModuleID),
+                     [](uint32_t V) { return V != 0; })) {
+
+            const GVSummaryMapTy &DefinedGlobals =
+                ModuleToDefinedGVSummaries.find(ModulePath)->second;
+
+            // Compute and store a bitcode module cache key.
+            J.CacheKey = computeLTOCacheKey(
+                Conf, CombinedIndex, ModulePath, ImportList, ExportList,
+                ResolvedODR, DefinedGlobals, CfiFunctionDefs, CfiFunctionDecls);
+
+            // Check if we have something in the cache.
+            auto CacheAddStreamExp = Cache(J.Task, J.CacheKey, J.ModuleID);
+            if (Error E = CacheAddStreamExp.takeError()) {
+              Err = joinErrors(std::move(*Err), std::move(E));
+            } else {
+              AddStreamFn &CacheAddStream = *CacheAddStreamExp;
+              if (!CacheAddStream) {
+                J.Cached = true; // Cache hit, mark the job as cached.
+                CachedJobs.fetch_add(1);
+              } else {
+                // Cache miss, save cache 'add stream' function for a later use.
+                J.CacheAddStream = std::move(CacheAddStream);
+              }
+            }
+          }
         },
         std::ref(J), std::ref(ImportList));
 
@@ -2417,6 +2456,9 @@ class OutOfProcessThinBackend : public CGThinBackend {
         for (const auto &J : Jobs) {
           assert(J.Task != 0);
 
+          if (!Cache.getCacheDirectoryPath().empty() && J.Cached)
+            continue;
+
           SmallVector<StringRef, 2> Inputs;
           SmallVector<StringRef, 1> Outputs;
 
@@ -2488,20 +2530,26 @@ class OutOfProcessThinBackend : public CGThinBackend {
         removeFile(JsonFile);
     });
 
-    SmallVector<StringRef, 3> Args = {DistributorPath};
-    llvm::append_range(Args, DistributorArgs);
-    Args.push_back(JsonFile);
-    std::string ErrMsg;
-    if (sys::ExecuteAndWait(Args[0], Args,
-                            /*Env=*/std::nullopt, /*Redirects=*/{},
-                            /*SecondsToWait=*/0, /*MemoryLimit=*/0, &ErrMsg)) {
-      return make_error<StringError>(
-          BCError + "distributor execution failed" +
-              (!ErrMsg.empty() ? ": " + ErrMsg + Twine(".") : Twine(".")),
-          inconvertibleErrorCode());
+    if (CachedJobs.load() < Jobs.size()) {
+      SmallVector<StringRef, 3> Args = {DistributorPath};
+      llvm::append_range(Args, DistributorArgs);
+      Args.push_back(JsonFile);
+      std::string ErrMsg;
+      if (sys::ExecuteAndWait(Args[0], Args,
+                              /*Env=*/std::nullopt, /*Redirects=*/{},
+                              /*SecondsToWait=*/0, /*MemoryLimit=*/0,
+                              &ErrMsg)) {
+        return make_error<StringError>(
+            BCError + "distributor execution failed" +
+                (!ErrMsg.empty() ? ": " + ErrMsg + Twine(".") : Twine(".")),
+            inconvertibleErrorCode());
+      }
     }
 
     for (auto &Job : Jobs) {
+      if (Cache.isValid() && !Job.CacheKey.empty())
+        if (Job.Cached)
+          continue;
       // Load the native object from a file into a memory buffer
       // and store its contents in the output buffer.
       auto ObjFileMbOrErr =
@@ -2512,15 +2560,32 @@ class OutOfProcessThinBackend : public CGThinBackend {
             BCError + "cannot open native object file: " +
                 Job.NativeObjectPath + ": " + EC.message(),
             inconvertibleErrorCode());
-      auto StreamOrErr = AddStream(Job.Task, Job.ModuleID);
-      if (Error Err = StreamOrErr.takeError())
-        report_fatal_error(std::move(Err));
-      auto &Stream = *StreamOrErr->get();
-      *Stream.OS << ObjFileMbOrErr->get()->getMemBufferRef().getBuffer();
-      if (Error Err = Stream.commit())
-        report_fatal_error(std::move(Err));
-    }
 
+      MemoryBufferRef ObjFileMbRef = ObjFileMbOrErr->get()->getMemBufferRef();
+      if (Cache.isValid() && Job.CacheAddStream) {
+        // Obtain a file stream for a storing a cache entry.
+        auto CachedFileStreamOrErr = Job.CacheAddStream(Job.Task, Job.ModuleID);
+        if (!CachedFileStreamOrErr)
+          return joinErrors(
+              CachedFileStreamOrErr.takeError(),
+              createStringError(inconvertibleErrorCode(),
+                                "Cannot get a cache file stream: %s",
+                                Job.NativeObjectPath.data()));
+        // Store a file buffer into the cache stream.
+        auto &CacheStream = *(CachedFileStreamOrErr->get());
+        *(CacheStream.OS) << ObjFileMbRef.getBuffer();
+        if (Error Err = CacheStream.commit())
+          return Err;
+      } else {
+        auto StreamOrErr = AddStream(Job.Task, Job.ModuleID);
+        if (Error Err = StreamOrErr.takeError())
+          report_fatal_error(std::move(Err));
+        auto &Stream = *StreamOrErr->get();
+        *Stream.OS << ObjFileMbRef.getBuffer();
+        if (Error Err = Stream.commit())
+          report_fatal_error(std::move(Err));
+      }
+    }
     return Error::success();
   }
 };
@@ -2535,12 +2600,12 @@ ThinBackend lto::createOutOfProcessThinBackend(
   auto Func =
       [=](const Config &Conf, ModuleSummaryIndex &CombinedIndex,
           const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
-          AddStreamFn AddStream, FileCache /*Cache*/) {
+          AddStreamFn AddStream, FileCache Cache) {
         return std::make_unique<OutOfProcessThinBackend>(
             Conf, CombinedIndex, Parallelism, ModuleToDefinedGVSummaries,
-            AddStream, OnWrite, ShouldEmitIndexFiles, ShouldEmitImportsFiles,
-            LinkerOutputFile, Distributor, DistributorArgs, RemoteCompiler,
-            RemoteCompilerArgs, SaveTemps);
+            AddStream, Cache, OnWrite, ShouldEmitIndexFiles,
+            ShouldEmitImportsFiles, LinkerOutputFile, Distributor,
+            DistributorArgs, RemoteCompiler, RemoteCompilerArgs, SaveTemps);
       };
   return ThinBackend(Func, Parallelism);
 }

vitalybuka · 2025-09-10T20:27:49Z

llvm/lib/LTO/LTO.cpp

  SmallVector<StringRef, 0> CodegenOptions;
  DenseSet<StringRef> CommonInputs;
-
+  std::atomic<uint64_t> CachedJobs{0};


size_t should be enough

Are you suggesting to change it to
std::atomic<size_t> CachedJobs{0};
or to
size_t CachedJobs{0};

This variable is changed in the thread pool. We cannot safely get rid of atomic because of a data race.

Vitaly, I was wondering if there are some other comments related to this PR?

changed to std::atomic<size_t> CachedJobs{0};

teresajohnson

Sorry for the slow review. Comments below.

Please also add llvm-lto2 based tests to llvm/test/ThinLTO/X86/dtlto, so that this gets exercised by LLVM tests.

teresajohnson · 2025-09-17T15:46:51Z

cross-project-tests/dtlto/dtlto-thinlto-cache.test

@@ -0,0 +1,66 @@
+REQUIRES: x86-registered-target, ld.lld
+
+# This test verifies that a cache populated by a ThinLTO link is not reused by a DTLTO link and vice versa.


Probably make this "by an in-process ThinLTO link", since DTLTO is also ThinLTO.

ditto other places below

Changed everywhere in the test. Hopefully, current wording is better. If not, I will re-write it.

teresajohnson · 2025-09-17T15:52:09Z

llvm/include/llvm/LTO/Config.h

  LLVM_ABI Error addSaveTemps(std::string OutputFileName,
                              bool UseInputModulePath = false,
                              const DenseSet<StringRef> &SaveTempsArgs = {});
+  mutable uint8_t Dtlto = 0;


bool? Also, add a comment about the need for this variable.

Changed its type to bool and added a comment.

This flag is used as one of parameters to calculate cache entries and to ensure that in-process cache and out-of-process (DTLTO) cache are distinguished.

It is passed as a part of const Config &Conf to a function that computes LTO Cache Key (computeLTOCacheKey).

Another option: instead of adding Dtlto to Config class, we could simply change the signature of computeLTOCacheKey and add Dtlto flag as an additional parameter to this function.

Do you think it will be better?

Having it in the Config seems fine. One other suggestion: move it up earlier in the struct definition to where there are other bool flags declared.

teresajohnson · 2025-09-17T15:53:37Z

llvm/lib/LTO/LTO.cpp

+         Saver.save(ObjFilePath.str()),
+         Saver.save(ObjFilePath.str() + ".thinlto.bc"),
+         {}, // Filled in by emitFiles below.
+         "",


Document constant values with comment e.g. /*CacheKey=*/

teresajohnson · 2025-09-17T15:54:22Z

llvm/lib/LTO/LTO.cpp

  SmallVector<StringRef, 0> CodegenOptions;
  DenseSet<StringRef> CommonInputs;
-
+  std::atomic<uint64_t> CachedJobs{0};


Add comment description

teresajohnson · 2025-09-17T16:10:00Z

llvm/lib/LTO/LTO.cpp

        for (const auto &J : Jobs) {
          assert(J.Task != 0);

+          if (!Cache.getCacheDirectoryPath().empty() && J.Cached)


When would Cache.getCacheDirectoryPath().empty() be true at the same time as J.Cached?

Agreed. Removed !Cache.getCacheDirectoryPath().empty()

Optionally add an assert that !Cache.getCacheDirectoryPath().empty().

teresajohnson · 2025-09-17T16:19:39Z

llvm/lib/LTO/LTO.cpp

    }

    for (auto &Job : Jobs) {
+      if (Cache.isValid() && !Job.CacheKey.empty())


Can Cache.isValid be false if CacheKey is not empty?

Agreed. Removed Cache.isValid from "if" condition.

Optionally add an assert that Cache.isValid()

teresajohnson · 2025-09-17T16:20:05Z

llvm/lib/LTO/LTO.cpp


    for (auto &Job : Jobs) {
+      if (Cache.isValid() && !Job.CacheKey.empty())
+        if (Job.Cached)


Combine with enclosing if check on prior line?

nit: remove extra parentheses around "(Job.Cached)" in combined if statement.

teresajohnson · 2025-09-17T16:20:24Z

llvm/lib/LTO/LTO.cpp

-    }

+      MemoryBufferRef ObjFileMbRef = ObjFileMbOrErr->get()->getMemBufferRef();
+      if (Cache.isValid() && Job.CacheAddStream) {


Can Cache.isValid be false if CacheAddStream is non-null?

Yes, if Job.CacheAddStream is non-null, Cache.isValid() is always true.

In this case I think it makes sense to leave Cache.IsValid as a part of if's condition for better understanding of the code, because there is an else statement below.

Job.CacheAddStream is non-null, when we have a cache miss. However, if we get to this point, we cannot have a cache hit. The only condition when we can execute "else" block below is when we don't have cache enabled, i.e. when Cache.isValid is false.

If I remove Cache.isValid() from the "if" condition the code will still be correct, but it won't be as readable.

I propose to leave it "as is". What do you think?

Or maybe will be clearer?
Replace this condition with Cache.isValid() and add a comment later saying that if we got to this point, we could only have cache miss.

if (Cache.isValid()) { // Cache hits are takes care of earlier. At this point, we could only have cache misses. ... } else { ... }

Yeah, I like your suggestion above better - then for code readability / documentation purposes add an "assert(Job.CacheAddStream);" in the if body.

teresajohnson · 2025-09-17T16:25:49Z

llvm/lib/LTO/LTO.cpp

-          BCError + "distributor execution failed" +
-              (!ErrMsg.empty() ? ": " + ErrMsg + Twine(".") : Twine(".")),
-          inconvertibleErrorCode());
+    if (CachedJobs.load() < Jobs.size()) {


Can you add a comment. I don't have a good understanding of what the CachedJobs count is for and how that works.

I added a comment with an explanation of its meaning at the point where CachedJobs variable has been defined. I also added an additional comment explaining an "if" statement.

teresajohnson · 2025-09-17T16:30:27Z

llvm/lib/LTO/LTO.cpp

              Err = std::move(E);
          }
+
+          if (Cache.isValid() &&


This lambda is getting pretty large. Can you refactor it into a runThinLTOBackendThread helper, and as much as possible structure it the same way as InProcessThinBackend::runThinLTOBackendThread ? Eventually it may make sense to move some of this into the base CGThinBackend, but at the very least, making the flow similar will help with comparing the functionality and code maintenance.

I will do it in a separate commit.

…ova-ekaterina/llvm-project into dtlto-cache-implementation

romanova-ekaterina · 2025-10-17T04:51:05Z

Sorry for the slow review. Comments below.

Please also add llvm-lto2 based tests to llvm/test/ThinLTO/X86/dtlto, so that this gets exercised by LLVM tests.

Sorry for the delayed response too. Your review came on the day I had a surgery. After that I was on short time leave from work for a month. Just returned back to work.

…cessThinBackend::runThinLTOBackendThread

romanova-ekaterina · 2025-11-04T01:15:14Z

@teresajohnson: Hi Teresa, do I need to do anything else to have this patch accepted? I think all the comments has been addressed. Thank you!

teresajohnson · 2025-11-04T16:03:55Z

llvm/lib/LTO/LTO.cpp

The return type is confusing, since you always return success but update Err to something other than success when there is an error. Can you change this function to return the error when one occurs and invoke joinErrors in the caller? I.e. structure like InProcessThinBackend::runThinLTOBackendThread. Suggestions to that effect below.

teresajohnson · 2025-11-04T16:07:32Z

llvm/lib/LTO/LTO.cpp

Move the stuff inside this if statement to caller, like at

llvm-project/llvm/lib/LTO/LTO.cpp

Lines 1571 to 1577 in bcb3d2f

if (E) {

std::unique_lock<std::mutex> L(ErrMu);

if (Err)

Err = joinErrors(std::move(*Err), std::move(E));

else

Err = std::move(E);

}

.

Then just return E here like at

llvm-project/llvm/lib/LTO/LTO.cpp

Lines 1522 to 1524 in bcb3d2f

if (auto E = emitFiles(ImportList, ModuleID, ModuleID.str()))

return E;

}

teresajohnson · 2025-11-04T16:11:24Z

llvm/lib/LTO/LTO.cpp

Just return E here like at

llvm-project/llvm/lib/LTO/LTO.cpp

Line 1539 in bcb3d2f

return Err;

, and move the stuff below here out of the else block.

teresajohnson · 2025-11-04T16:13:20Z

llvm/lib/LTO/LTO.cpp

Change this to be like

llvm-project/llvm/lib/LTO/LTO.cpp

Line 1526 in bcb3d2f

if (!Cache.isValid() || !CombinedIndex.modulePaths().count(ModuleID) ||

(i.e. reverse the conditions to be (!Cache.isValid() || ! ....), and return success early if so. Then the below code can be unnested from the if.

teresajohnson · 2025-11-04T16:15:07Z

llvm/lib/LTO/LTO.cpp

-    }

+      MemoryBufferRef ObjFileMbRef = ObjFileMbOrErr->get()->getMemBufferRef();
+      if (Cache.isValid() && Job.CacheAddStream) {


Yeah, I like your suggestion above better - then for code readability / documentation purposes add an "assert(Job.CacheAddStream);" in the if body.

teresajohnson · 2025-11-04T16:17:38Z

llvm/include/llvm/LTO/Config.h

  LLVM_ABI Error addSaveTemps(std::string OutputFileName,
                              bool UseInputModulePath = false,
                              const DenseSet<StringRef> &SaveTempsArgs = {});
+  mutable uint8_t Dtlto = 0;


Having it in the Config seems fine. One other suggestion: move it up earlier in the struct definition to where there are other bool flags declared.

teresajohnson · 2025-11-04T16:18:41Z

llvm/lib/LTO/LTO.cpp

        for (const auto &J : Jobs) {
          assert(J.Task != 0);

+          if (!Cache.getCacheDirectoryPath().empty() && J.Cached)


Optionally add an assert that !Cache.getCacheDirectoryPath().empty().

teresajohnson · 2025-11-04T16:18:59Z

llvm/lib/LTO/LTO.cpp

    }

    for (auto &Job : Jobs) {
+      if (Cache.isValid() && !Job.CacheKey.empty())


Optionally add an assert that Cache.isValid()

teresajohnson · 2025-11-04T16:20:15Z

llvm/lib/LTO/LTO.cpp


    for (auto &Job : Jobs) {
+      if (Cache.isValid() && !Job.CacheKey.empty())
+        if (Job.Cached)


nit: remove extra parentheses around "(Job.Cached)" in combined if statement.

teresajohnson · 2025-11-04T16:38:24Z

Please also add llvm-lto2 based tests to llvm/test/ThinLTO/X86/dtlto, so that this gets exercised by LLVM tests.

This one was missed - the change needs some llvm-lto2 based tests as well.

romanova-ekaterina · 2025-11-07T13:40:34Z

Please also add llvm-lto2 based tests to llvm/test/ThinLTO/X86/dtlto, so that this gets exercised by LLVM tests.

This one was missed - the change needs some llvm-lto2 based tests as well.

I am working on the llvm-lto2 based test and I will post it in a separate commit.

teresajohnson

lgtm with one minor comment typo noted below.

teresajohnson · 2025-11-16T05:43:13Z

llvm/lib/LTO/LTO.cpp

s/takes/taken/

llvm-ci · 2025-11-17T12:29:41Z

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building cross-project-tests,llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/35362

Here is the relevant piece of the build log for the reference

Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: functionalities/thread/jump/TestThreadJump.py (265 of 3318)
PASS: lldb-api :: functionalities/thread/ignore_suspended/TestIgnoreSuspendedThread.py (266 of 3318)
PASS: lldb-api :: functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py (267 of 3318)
PASS: lldb-api :: functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py (268 of 3318)
PASS: lldb-unit :: Host/./HostTests/92/141 (269 of 3318)
PASS: lldb-shell :: SymbolFile/DWARF/debug-types-expressions.test (270 of 3318)
PASS: lldb-api :: functionalities/inferior-changed/TestInferiorChanged.py (271 of 3318)
PASS: lldb-api :: functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py (272 of 3318)
PASS: lldb-api :: functionalities/plugins/command_plugin/TestPluginCommands.py (273 of 3318)
PASS: lldb-api :: lang/c/conflicting-symbol/TestConflictingSymbol.py (274 of 3318)
FAIL: lldb-api :: tools/lldb-dap/output/TestDAP_output.py (275 of 3318)
******************** TEST 'lldb-api :: tools/lldb-dap/output/TestDAP_output.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib --cmake-build-type Release -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/output -p TestDAP_output.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision 3ee54a6b992c6053726764905030946f8bc10cd0)
  clang revision 3ee54a6b992c6053726764905030946f8bc10cd0
  llvm revision 3ee54a6b992c6053726764905030946f8bc10cd0
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'pdb', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/output
runCmd: settings clear --all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output:

This patch implements DTLTO cache. DTLTO cache is implemented the same way as ThinLTO cache. In fact the same class Cache is used for both of them. Because parameters for codegen are different for DTLTO and ThinLTO (DTLTO codegen is done by invoking clang and its codegen parameters are not fully synchronized with codegen parameters used by LTO backend). The object files generated by DTLTO and ThinLTO might be different and shouldn't be mixed. If ThinLTO and DTLTO share the same cache directory, the cache file won't interfere with each other. I added a couple of test files in cross-project-test/dtlto directory, but if more tests are required for initial implementation, I could add them.

romanova-ekaterina force-pushed the dtlto-cache-implementation branch 2 times, most recently from c270018 to 514ed5f Compare September 3, 2025 10:43

romanova-ekaterina force-pushed the dtlto-cache-implementation branch from 514ed5f to 1ecbb72 Compare September 4, 2025 06:13

llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Sep 4, 2025

romanova-ekaterina requested review from MaskRay, bd1976bris, cachemeifyoucan, teresajohnson and vitalybuka September 4, 2025 08:34

bd1976bris mentioned this pull request Sep 4, 2025

Integrated Distributed ThinLTO (DTLTO): Design Overview #126654

Open

romanova-ekaterina changed the title ~~[DTLTO] [Clang] Initial DTLTO cache implementation~~ [DTLTO] [LLVM] Initial DTLTO cache implementation Sep 5, 2025

vitalybuka reviewed Sep 10, 2025

View reviewed changes

vitalybuka self-requested a review September 10, 2025 20:28

teresajohnson reviewed Sep 17, 2025

View reviewed changes

romanova-ekaterina and others added 5 commits October 8, 2025 00:55

Merge branch 'main' into dtlto-cache-implementation

3e5c202

Merge branch 'main' into dtlto-cache-implementation

2bfae1f

Merge branch 'main' into dtlto-cache-implementation

ac2176e

Updated first round of code review comments

4c31381

Merge branch 'dtlto-cache-implementation' of https://github.com/roman…

9caf360

…ova-ekaterina/llvm-project into dtlto-cache-implementation

Rewrote rather large lambda and restructured it the same way as InPro…

9a1cead

…cessThinBackend::runThinLTOBackendThread

teresajohnson reviewed Nov 4, 2025

View reviewed changes

romanova-ekaterina and others added 2 commits November 7, 2025 02:58

Updated second rount of code review comments

1b6df1c

Merge branch 'main' into dtlto-cache-implementation

81597e5

Added llvm-lto2 test

a298e49

teresajohnson approved these changes Nov 16, 2025

View reviewed changes

llvm/lib/LTO/LTO.cpp Outdated

Copy link

Contributor

teresajohnson Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/takes/taken/

Fixed spelling mistake

deddc10

romanova-ekaterina merged commit 3ee54a6 into llvm:main Nov 17, 2025
10 checks passed

		@@ -0,0 +1,66 @@
		REQUIRES: x86-registered-target, ld.lld

		# This test verifies that a cache populated by a ThinLTO link is not reused by a DTLTO link and vice versa.

	if (E) {
	std::unique_lock<std::mutex> L(ErrMu);
	if (Err)
	Err = joinErrors(std::move(*Err), std::move(E));
	else
	Err = std::move(E);
	}

	if (auto E = emitFiles(ImportList, ModuleID, ModuleID.str()))
	return E;
	}

[DTLTO] [LLVM] Initial DTLTO cache implementation #156433

[DTLTO] [LLVM] Initial DTLTO cache implementation #156433

Conversation

romanova-ekaterina commented Sep 2, 2025

Uh oh!

github-actions bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romanova-ekaterina Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teresajohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romanova-ekaterina Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

github-actions bot commented Sep 2, 2025 •

edited

Loading

romanova-ekaterina Sep 11, 2025 •

edited

Loading

romanova-ekaterina Oct 22, 2025 •

edited

Loading

romanova-ekaterina Oct 20, 2025 •

edited

Loading