[PATCH] D87966: [ThinLTO] Re-order modules for optimal multi-threaded processing

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

It would be great to test that the ordering of the files in the object file list doesn't change with the write indexes backend. Unfortunately, currently llvm-lto2 doesn't have an option to trigger this output file, but you could use either the gold plugin or lld. There is an existing gold plugin test that does test this output file:
llvm/test/tools/gold/X86/thinlto_emit_linked_objects.ll

It looks like that test would have failed if the object files had been reordered, but it looks like the first one listed is bigger so due to luck we ended up with the same ordering with your original patch. You could probably make a new version of this test that doesn't use --start-lib/--end-lib but rather just passes both bitcode objects to gold normally, and reorder the order the two bitcode files are passed so the Input version which appears smaller is passed first, then confirm that you get the expected ordering in the object list file %t3 (which would be the order that they were passed to gold). Would be good to confirm that without the new guard against reordering with the write indexes backend the ordering gets switched, which is what we want to prevent.



================
Comment at: llvm/lib/LTO/LTO.cpp:1468
 
+  if (RegularLTO.ParallelCodeGenParallelismLevel == 1 ||
+      BackendProc->kind() == ThinBackendProc::WriteIndexesKind) {
----------------
The ParallelCodeGenParallelismLevel is unrelated to the number of threads we will be using for ThinLTO backends (rather it is related to regular LTO code generation). For the latter you would need to do something like call getThreadCount() on the BackendThreadPool on the InProcessThinBackend.


================
Comment at: llvm/lib/LTO/LTO.cpp:1469
+  if (RegularLTO.ParallelCodeGenParallelismLevel == 1 ||
+      BackendProc->kind() == ThinBackendProc::WriteIndexesKind) {
+    // Process the modules in the order they were provided.
----------------
Needs comment as to why we are not doing this for WriteIndexesKind


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87966/new/

https://reviews.llvm.org/D87966