[ThinLTO] Option to control path of distributed backend files

[tejohnson]

Summary

Add new thinlto-prefix-replace plugin option to control where files
for a distributed backend (the individual index files and optional
imports files) are created.

If specified, expects a string of the form "oldprefix:newprefix", and
instead of generating these files in the same directory path as the
corresponding bitcode file, will use a path formed by replacing the
bitcode file's path prefix matching oldprefix with newprefix.

Depends on D19636.

[mehdi_amini]

test/tools/gold/X86/thinlto_prefix_replace.ll

4

Deserve a comment

tools/gold/gold-plugin.cpp

198

What is the motivation for that?

[tejohnson]

test/tools/gold/X86/thinlto_prefix_replace.ll

4

Will add.

tools/gold/gold-plugin.cpp

198

In our particular instance it helps enable sharing of bitcode files across builds of different libraries or binaries. I.e. the bitcode files, the result of the first compile step, are output into one tree. When the ThinLTO link and backend steps execute, we want the resulting index/imports files to go into a different tree specific to the target binary/library. That way the same bitcode tree can be used as input across builds for multiple libraries/binaries. But this generally seems like useful functionality to allow flexibility in the build system.

[tejohnson]

Add comment to test. Rebase.

[mehdi_amini]

Can this be exposed to llvm-lto as well? It'll help testing when refactoring everything behind a new API.

test/tools/gold/X86/thinlto_prefix_replace.ll

14

I think you can just write ls %T/newpath/thinlto_prefix_replace.o.thinlto.bc without any FileCheck, it should be enough to test the existence of the file.

[tejohnson]

In D19644#421629, @joker.eph wrote:

Can this be exposed to llvm-lto as well? It'll help testing when refactoring everything behind a new API.

I wasn't sure the best way to do that initially, but thinking some more I think this is what I can do: In D19556 (individual index files) I will sink the lifetime of the raw_fd_ostream into WriteIndexToFile (similar to how it is done within EmitImportsFiles in D19636). Then I can move getOutputModulePath into FunctionImport.cpp as well, and just pass the new and old prefix into those routines and do the replacement there before opening the files.

test/tools/gold/X86/thinlto_prefix_replace.ll

14

Ok, will give that a try.

[tejohnson]

In D19644#421867, @tejohnson wrote:

In D19644#421629, @joker.eph wrote:

Can this be exposed to llvm-lto as well? It'll help testing when refactoring everything behind a new API.

I wasn't sure the best way to do that initially, but thinking some more I think this is what I can do: In D19556 (individual index files) I will sink the lifetime of the raw_fd_ostream into WriteIndexToFile (similar to how it is done within EmitImportsFiles in D19636). Then I can move getOutputModulePath into FunctionImport.cpp as well, and just pass the new and old prefix into those routines and do the replacement there before opening the files.

I remembered why I didn't do this - what I describe above doesn't work since WriteIndexToFile is in the bitcode writer, and I don't want to add a dependence from it to the function importer or vice versa. It's difficult to sink the code here, which is just doing path and file operations, into the function importer since the result is used both by EmitImports (in the function importer) and for WriteIndexToFile (in the bitcode writer).

What I did instead was to extract the path manipulation done here to replace the matching path prefix with a new prefix into a new path method in libSupport (and added unit tests). I then invoke that both from gold-plugin as well as from llvm-lto. There is some code duplicated between gold-plugin and llvm-lto but it is now pretty minimal (just invoking some basic option parsing and the path support library routines). I added a companion llvm-lto test as well.

[tejohnson]

Extract path manipulation into libSupport path helper, add unit tests. Add
similar support to llvm-lto along with test.

[tejohnson]

Forgot to add new llvm-lto based test in previous update, add it here.

[tejohnson]

Ping.
Thanks,
Teresa

[mehdi_amini]

LGTM, with some suggestions inline (no need to review again)

lib/Support/Path.cpp

533 ↗

(On Diff #56910)

You mentioned as precondition in the doc:

 @param Path If \a Path starts with \a OldPrefix modify to instead
///        start with \a NewPrefix.

Here you're still doing the check. I'd either assert or update the comment.

539 ↗

(On Diff #56910)

If OldPrefix.size() == NewPrefix.size() we have a possible fast-path in-place.
(the unittest should also be updated to test this case)

tools/gold/gold-plugin.cpp

1216

This is a user-supplied command line flag, if you assert he I'd expect it to be appropriately validated with a report_fatal_error (or equivalent) at parse time (line 226 above)

[mehdi_amini]

tools/llvm-lto/llvm-lto.cpp

331 ↗

(On Diff #56910)

The duplication with the gold plugin isn't great for these two functions.

[tejohnson]

lib/Support/Path.cpp

533 ↗

(On Diff #56910)

I think the code matches the description, which indicates that we only do the replacement if Path starts with OldPrefix. Here we are skipping the replacement if Path does not start with OldPrefix.

539 ↗

(On Diff #56910)

I can do that.

tools/gold/gold-plugin.cpp

1216

Ok

tools/llvm-lto/llvm-lto.cpp

331 ↗

(On Diff #56910)

Agree, but I couldn't figure out a good way to avoid that. They didn't seem general enough to move into lib/Support either.

[mehdi_amini]

lib/Support/Path.cpp

533 ↗

(On Diff #56910)

Somehow I missed the "If" in the comment when I copy/pasted... Nevermind!

tools/llvm-lto/llvm-lto.cpp

331 ↗

(On Diff #56910)

It is not critical if there was no convenient way.

test/tools/gold/X86/thinlto_prefix_replace.ll

+; Check that changing the output path via thinlto-prefix-replace works
+; RUN: mkdir -p %T/oldpath
+; RUN: opt -module-summary %s -o %T/oldpath/thinlto_prefix_replace.o
+; RUN: rm -f %T/newpath/thinlto_prefix_replace.o.thinlto.bc
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=thinlto-index-only \
+; RUN:    --plugin-opt=thinlto-prefix-replace="%T/oldpath/:%T/newpath/" \
+; RUN:    -shared %T/oldpath/thinlto_prefix_replace.o -o %T/thinlto_prefix_replace
+; RUN: ls -l %T/newpath/thinlto_prefix_replace.o.thinlto.bc | FileCheck %s
+; CHECK: thinlto_prefix_replace.o.thinlto.bc
+
+define void @f() {
+entry:
+  ret void
+}

tools/gold/gold-plugin.cpp

#endif
   // the import decisions, and exit afterwards. The assumption is
   // that the build system will launch the backend processes.
   static bool thinlto_index_only = false;
   // If true, when generating individual index files for distributed backends,
   // also generate a "${bitcodefile}.imports" file at the same location for each
   // bitcode file, listing the files it imports from in plain text. This is to
   // support distributed build file staging.
   static bool thinlto_emit_imports_files = false;
+  // Option to control where files for a distributed backend (the individual
+  // index files and optional imports files) are created.
+  // If specified, expects a string of the form "oldprefix:newprefix", and
+  // instead of generating these files in the same directory path as the
+  // corresponding bitcode file, will use a path formed by replacing the
+  // bitcode file's path prefix matching oldprefix with newprefix.
+  static std::string thinlto_prefix_replace;
   // Additional options to pass into the code generator.
   // Note: This array will contain all plugin options which are not claimed
   // as plugin exclusive to pass to the code generator.
   // For example, "generate-api-file" and "as"options are for the plugin
   // use only and will not be passed.
   static std::vector<const char *> extra;
 
   static void process_plugin_option(const char *opt_)
static void process_plugin_option(const char *opt_)
     } else if (opt == "disable-output") {
       TheOutputType = OT_DISABLE;
     } else if (opt == "thinlto") {
       thinlto = true;
     } else if (opt == "thinlto-index-only") {
       thinlto_index_only = true;
     } else if (opt == "thinlto-emit-imports-files") {
       thinlto_emit_imports_files = true;
+    } else if (opt.startswith("thinlto-prefix-replace=")) {
+      thinlto_prefix_replace = opt.substr(strlen("thinlto-prefix-replace="));
     } else if (opt.size() == 2 && opt[0] == 'O') {
       if (opt[1] < '0' || opt[1] > '3')
         message(LDPL_FATAL, "Optimization level must be between 0 and 3");
       OptLevel = opt[1] - '0';
     } else if (opt.startswith("jobs=")) {
       if (StringRef(opt_ + 5).getAsInteger(10, Parallelism))
         message(LDPL_FATAL, "Invalid parallelism level: %s", opt_ + 5);
     } else if (opt == "disable-verify") {
for (claimed_file &F : Modules) {
                          NewFilename.c_str(), TempOutFile);
     }
   }
 
   for (auto &Task : Tasks)
     Task.cleanup();
 }
 
+/// Returns the path formed by replacing the path prefix of \p OrigModulePath
+/// that matches \p OrigPrefix with \p NewPrefix. The resulting directory is
+/// created if it doesn't exist.
+static std::string getOutputModulePath(StringRef OrigModulePath,
+                                       std::string OrigPrefix,
+                                       std::string NewPrefix) {
+  if (options::thinlto_prefix_replace.empty() ||
+      !OrigModulePath.startswith(OrigPrefix))
+    return OrigModulePath;
+
+  // First compute the new module path by stripping the old prefix and
+  // prepending the new prefix.
+  StringRef ModuleRelPath = OrigModulePath.substr(OrigPrefix.size());
+  std::string NewModulePath = NewPrefix + ModuleRelPath.str();
+
+  // Now create the directory if it doesn't exist.
+  std::string NewDir = NewPrefix;
+  // See if ModuleRelPath (the part of the path not replaced above) contains
+  // a path. If so, append it to the NewPrefix to get the directory.
+  size_t last_dir_sep = ModuleRelPath.rfind('/');
+  if (last_dir_sep != StringRef::npos)
+    NewDir += ModuleRelPath.slice(0, last_dir_sep).str();
+  // Make sure the new directory exists, creating it if necessary.
+  if (std::error_code EC = llvm::sys::fs::create_directories(NewDir))
+    llvm::errs() << "warning: could not create directory '" << NewDir
+                 << "': " << EC.message() << '\n';
+
+  return NewModulePath;
+}
+
 /// Perform ThinLTO link, which creates the combined index file.
 /// Also, either launch backend threads or (under thinlto-index-only)
 /// emit individual index files for distributed backends and exit.
 static ld_plugin_status thinLTOLink(raw_fd_ostream *ApiFile) {
   ModuleSummaryIndex CombinedIndex;
   uint64_t NextModuleId = 0;
   for (claimed_file &F : Modules) {
     PluginInputFile InputFile(F.handle);
if (options::thinlto_index_only) {
     // from gold as well, in which case this will be moved out of the
     // thinlto_index_only handling, and the function importer will be invoked
     // directly using the Lists.
     StringMap<FunctionImporter::ImportMapTy> ImportLists(NextModuleId);
     StringMap<FunctionImporter::ExportSetTy> ExportLists(NextModuleId);
     ComputeCrossModuleImport(CombinedIndex, ModuleToDefinedGVSummaries,
                              ImportLists, ExportLists);
 
+    // If the thinlto-prefix-replace option was specified, parse it and
+    // extract the old and new prefixes.
+    StringRef PrefixReplace = options::thinlto_prefix_replace;
+    assert(PrefixReplace.empty() || PrefixReplace.find(":") != StringRef::npos);
+    StringRef PrefixReplaceStr = PrefixReplace;
+    std::pair<StringRef, StringRef> Split = PrefixReplaceStr.split(":");
+    std::string OrigPrefix = Split.first.str();
+    std::string NewPrefix = Split.second.str();
+
     // For each input bitcode file, generate an individual index that
     // contains summaries only for its own global values, and for any that
     // should be imported.
     for (claimed_file &F : Modules) {
       PluginInputFile InputFile(F.handle);
       std::error_code EC;
-      raw_fd_ostream OS((Twine(InputFile.file().name) + ".thinlto.bc").str(),
-                        EC, sys::fs::OpenFlags::F_None);
+
+      std::string NewModulePath =
+          getOutputModulePath(InputFile.file().name, OrigPrefix, NewPrefix);
+      raw_fd_ostream OS((Twine(NewModulePath) + ".thinlto.bc").str(), EC,
+                        sys::fs::OpenFlags::F_None);
       if (EC)
         message(LDPL_FATAL, "Unable to open %s.thinlto.bc for writing: %s",
-                InputFile.file().name, EC.message().c_str());
+                NewModulePath.c_str(), EC.message().c_str());
       // Build a map of module to the GUIDs and summary objects that should
       // be written to its index.
       // Use a std::map instead of StringMap to get stable order for
       // bitcode emission.
       std::map<StringRef, GVSummaryMapTy> ModuleToSummariesForIndex;
       // Include all summaries from the importing module.
       ModuleToSummariesForIndex[InputFile.file().name] =
           ModuleToDefinedGVSummaries[InputFile.file().name];
       auto ModuleImports = ImportLists.find(InputFile.file().name);
       if (ModuleImports != ImportLists.end()) {
         // Include summaries for imports.
         for (auto &ILI : ModuleImports->second) {
           auto &SummariesForIndex = ModuleToSummariesForIndex[ILI.first()];
           auto &DefinedGVSummaries = ModuleToDefinedGVSummaries[ILI.first()];
           for (auto &GI : ILI.second) {
             SummariesForIndex[GI.first] = DefinedGVSummaries[GI.first];
           }
         }
       }
       if (options::thinlto_emit_imports_files) {
-        raw_fd_ostream ImportsOS(
-            (Twine(InputFile.file().name) + ".imports").str(), EC,
+        raw_fd_ostream ImportsOS((Twine(NewModulePath) + ".imports").str(), EC,
                                  sys::fs::OpenFlags::F_None);
         if (EC)
           message(LDPL_FATAL, "Unable to open %s.imports",
-                  InputFile.file().name, EC.message().c_str());
+                  NewModulePath.c_str(), EC.message().c_str());
         if (ModuleImports != ImportLists.end())
           for (auto &ILI : ModuleImports->second)
             ImportsOS << ILI.first() << "\n";
       }
       WriteIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
     }
 
     cleanup_hook();