[ThinLTO] Fix temp file dumping, enable via llvm-lto and test it

[tejohnson]

Summary

Fixed a bug in ThinLTOCodeGenerator's temp file dumping. The Twine
needs to be passed directly as an argument, not saved.

It doesn't seem there are any consumers of this, so I added a new option
to llvm-lto to enable saving of temp files during ThinLTO, and augmented
a test to use it to check post-import but pre-opt bitcode.

[mehdi_amini]

It doesn't seem there are any consumers of this

I noticed this last week looking at the coverage report on llvm.org. Thanks for adding this!

lib/LTO/ThinLTOCodeGenerator.cpp

81 ↗

(On Diff #68083)

Damn auto...

82 ↗

(On Diff #68083)

To avoid the code duplication, can we keep the SaveTempPath but declared as follows?

std::string SaveTempPath = (TempDir + llvm::utostr(count) + Suffix).str();

83 ↗

(On Diff #68083)

I probably added the .str() here to fix a compile error, it should have hinted me that something was wrong!

[mehdi_amini]

(LGTM of course)

[tejohnson]

lib/LTO/ThinLTOCodeGenerator.cpp

82 ↗

(On Diff #68083)

Yep, done. Will commit version with that change.

llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

 }
 
 // Simple helper to save temporary files for debug.
 static void saveTempBitcode(const Module &TheModule, StringRef TempDir,
                             unsigned count, StringRef Suffix) {
   if (TempDir.empty())
     return;
   // User asked to save temps, let dump the bitcode file after import.
-  auto SaveTempPath = TempDir + llvm::utostr(count) + Suffix;
+  std::string SaveTempPath = (TempDir + llvm::utostr(count) + Suffix).str();
   std::error_code EC;
-  raw_fd_ostream OS(SaveTempPath.str(), EC, sys::fs::F_None);
+  raw_fd_ostream OS(SaveTempPath, EC, sys::fs::F_None);
   if (EC)
     report_fatal_error(Twine("Failed to open ") + SaveTempPath +
                        " to save optimized bitcode\n");
   WriteBitcodeToFile(&TheModule, OS, /* ShouldPreserveUseListOrder */ true);
 }
 
 static const GlobalValueSummary *
 getFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList) {

llvm/trunk/test/ThinLTO/X86/linkonce_resolution_comdat.ll

 ; This test ensures that we drop the preempted copy of @f from %t2.bc from its
 ; comdat after making it available_externally. If not we would get a
 ; verification error.
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g
+; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
+
+; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT1
+; RUN: llvm-dis %t3.1.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT2
+; Copy from first module is prevailing and converted to weak_odr, copy
+; from second module is preempted and converted to available_externally and
+; removed from comdat.
+; IMPORT1: define weak_odr i32 @f(i8*) unnamed_addr comdat($c1) {
+; IMPORT2: define available_externally i32 @f(i8*) unnamed_addr {
 
 ; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
 ; NM1: W f
 
 ; RUN: llvm-nm -o - < %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM2
 ; f() would have been turned into available_externally since it is preempted,
 ; and inlined into g()
 ; NM2-NOT: f

llvm/trunk/tools/llvm-lto/llvm-lto.cpp

 static cl::opt<std::string> ThinLTOModuleId(
     "thinlto-module-id",
     cl::desc("For the module ID for the file to process, useful to "
              "match what is in the index."));
 
 static cl::opt<std::string>
     ThinLTOCacheDir("thinlto-cache-dir", cl::desc("Enable ThinLTO caching."));
 
+static cl::opt<std::string> ThinLTOSaveTempsPrefix(
+    "thinlto-save-temps",
+    cl::desc("Save ThinLTO temp files using filenames created by adding "
+             "suffixes to the given file path prefix."));
+
 static cl::opt<bool>
     SaveModuleFile("save-merged-module", cl::init(false),
                    cl::desc("Write merged LTO module to file before CodeGen"));
 
 static cl::list<std::string> InputFilenames(cl::Positional, cl::OneOrMore,
                                             cl::desc("<input bitcode files>"));
 
 static cl::opt<std::string> OutputFilename("o", cl::init(""),
for (unsigned i = 0; i < InputFilenames.size(); ++i) {
       auto &Filename = InputFilenames[i];
       StringRef CurrentActivity = "loading file '" + Filename + "'";
       auto InputOrErr = MemoryBuffer::getFile(Filename);
       error(InputOrErr, "error " + CurrentActivity);
       InputBuffers.push_back(std::move(*InputOrErr));
       ThinGenerator.addModule(Filename, InputBuffers.back()->getBuffer());
     }
 
+    if (!ThinLTOSaveTempsPrefix.empty())
+      ThinGenerator.setSaveTempsDir(ThinLTOSaveTempsPrefix);
     ThinGenerator.run();
 
     auto &Binaries = ThinGenerator.getProducedBinaries();
     if (Binaries.size() != InputFilenames.size())
       report_fatal_error("Number of output objects does not match the number "
                          "of inputs");
 
     for (unsigned BufID = 0; BufID < Binaries.size(); ++BufID) {