Enable debug fission for thinLTO linked via gold-plugin

[yunlian]

Summary

This enables debug fission on implicit ThinLTO when linked with gold. It will put the .dwo files in a directory specified by user.

[pcc]

Please upload your patch with context.

include/llvm/LTO/Config.h

77 ↗

(On Diff #139468)

Rename to match coding conventions: DwoDir.

include/llvm/LTO/LTO.h

169 ↗

(On Diff #139468)

Remove this field.

lib/LTO/LTOBackend.cpp

439 ↗

(On Diff #139468)

The module identifier is not enough to uniquely identify a .dwo file, as there could be multiple inputs with the same identifier. It could also contain slashes (which would require creating parent directories) or other characters which are invalid to use in file names. Probably the simplest thing to do is use the task identifier.

488 ↗

(On Diff #139468)

Move all of this logic into codegen; that way, you don't need to pass a file name back. If fission is enabled we should call addPassesToEmitFile on a temporary file, and then have objcopy operate on that file and finally open it and write its contents to Stream->OS.

[yunlian]

lib/LTO/LTOBackend.cpp

439 ↗

(On Diff #139468)

I think it should be good enough in normal cases.
Below are some example identifier when I link chrome.

./obj/device/bluetooth/public/mojom/mojom/uuid.mojom.o
./obj/device/bluetooth/libbluetooth.a.llvm.19039330.bluetooth_adapter_client.cc

For .o, it is the name (path included) of the input bitecode. For archives, it is the name of the archive appended by offset and the source file name.

The only thing that it does not work is that, the same input files are shared by different binaries and the binaries are linked in the same time.

The module name implies that we may need to create sub directories to store the .dwo files. I think that should be fine.

[pcc]

lib/LTO/LTOBackend.cpp

439 ↗

(On Diff #139468)

I think it should be good enough in normal cases.

Yes, but the task identifier is good enough in all cases, and it requires less code.

[yunlian]

lib/LTO/LTOBackend.cpp

439 ↗

(On Diff #139468)

I use the module identifier without directory information and the Task as the .dwo file name.

[pcc]

A reminder to add context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Test cases please. Also I think you will need to add the dwo path and the task id to the cache key if split dwarf is enabled.

include/llvm/LTO/Config.h

76 ↗

(On Diff #139607)

Drop the "when using ThinLTO" part because this should also work with regular LTO.

lib/LTO/LTOBackend.cpp

292 ↗

(On Diff #139607)

if (auto EC = sys::...

305 ↗

(On Diff #139607)

Indentation

308 ↗

(On Diff #139607)

Do you need this line?

310 ↗

(On Diff #139607)

Remove braces and if (auto EC = sys::...

311 ↗

(On Diff #139607)

Include EC.message() in error.

323–324 ↗

(On Diff #139607)

Add error handling.

327 ↗

(On Diff #139607)

Add error handling.

[pcc]

In D44792#1047010, @pcc wrote:

A reminder to add context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Test cases please. Also I think you will need to add the dwo path and the task id to the cache key if split dwarf is enabled.

Please address.

lib/LTO/LTOBackend.cpp

294 ↗

(On Diff #139669)

This can just be llvm::raw_fd_ostream OS(FD, true); because you aren't transferring the ownership anywhere.

328 ↗

(On Diff #139669)

I wouldn't bother with the arguments, I'd probably say something like Failed to extract dwo: exit code foo. Same below.

[yunlian]

In D44792#1047010, @pcc wrote:

A reminder to add context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Test cases please. Also I think you will need to add the dwo path and the task id to the cache key if split dwarf is enabled.

I added the dwo dir to cache key.
About the test cases, I am not sure how to do that in my case.
Ideally, it should build a binary and I can check the output of readelf.
But this needs clang. Can we call clang in a unittest of llvm?

[pcc]

In D44792#1050454, @yunlian wrote:

In D44792#1047010, @pcc wrote:

A reminder to add context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Test cases please. Also I think you will need to add the dwo path and the task id to the cache key if split dwarf is enabled.

I added the dwo dir to cache key.
About the test cases, I am not sure how to do that in my case.
Ideally, it should build a binary and I can check the output of readelf.
But this needs clang. Can we call clang in a unittest of llvm?

You shouldn't need clang to test ThinLTO. See for example any of the tests in test/tools/gold/X86/thinlto*.ll.

[yunlian]

You shouldn't need clang to test ThinLTO. See for example any of the tests in test/tools/gold/X86/thinlto*.ll.

Thanks, It seems I can to pass -shared to gold to avoid invoking clang. A test case was uploaded.

[pcc]

lib/LTO/LTO.cpp

139 ↗

(On Diff #140854)

You don't need the if condition here.

test/tools/gold/X86/split-dwarf.ll

1 ↗

(On Diff #140854)

I don't think this is necessary, as long as you use LLVM tools.

3 ↗

(On Diff #140854)

Prefer to specify absolute paths to commands to avoid depending on the current directory.

7 ↗

(On Diff #140854)

We generally don't depend on GNU binutils in our tests. You probably want to pass the path to the just-built llvm-objcopy here.

9 ↗

(On Diff #140854)

This test should start with textual IR instead of a .bc file. Use the opt command to create a .bc file from your test input and then pass that to gold.

15 ↗

(On Diff #140854)

Similarly, don't depend on binutils here. Can this be tested with llvm-dwarfdump?

[yunlian]

lib/LTO/LTO.cpp

139 ↗

(On Diff #140854)

Will do

test/tools/gold/X86/split-dwarf.ll

1 ↗

(On Diff #140854)

The reason I want to exclude windows platform is I want to use 'which' command line, if I can remove that, I will remove this line too.

7 ↗

(On Diff #140854)

How can I get the path of the llvm-objcopy? I can run llvm-* command line inside llvm-lit without specifying the path. But this is one is a little bit tricky, it needs the full path of the objcopy.

9 ↗

(On Diff #140854)

If I use llvm-as or opt to create a .bc file from .ll and do the same test, the .dwo file name changes from split-dwarf.dwo to ld-temp.dwo. Is that expected?

15 ↗

(On Diff #140854)

llvm-dwarfdump works here.
Will do.

[pcc]

test/tools/gold/X86/split-dwarf.ll

7 ↗

(On Diff #140854)

Create a ToolSubst for %llvm-objcopy in llvm/test/lit.cfg.py and use it here.

9 ↗

(On Diff #140854)

Your .bc files don't have a summary, so they are being compiled with full LTO instead of ThinLTO. Pass -module-summary to opt to get ThinLTO object files.

Come to think of it, we probably want to test both full and ThinLTO. Can you construct a test with one full LTO input file and one ThinLTO?

[yunlian]

test/tools/gold/X86/split-dwarf.ll

3 ↗

(On Diff #140854)

It seems that %t is an absolute path already.

[pcc]

test/tools/gold/X86/split-dwarf.ll

3 ↗

(On Diff #140854)

Yes but you are passing a relative path as dwo_dir. Pass %t/dwo_dir instead and remove this line.

[pcc]

test/lit.cfg.py

139 ↗

(On Diff #140999)

%llvm-objcopy (and elsewhere)

Can you use FindTool here?

[pcc]

test/lit.cfg.py

139 ↗

(On Diff #140999)

Looks like the %llvm-objcopy part isn't done.

[yunlian]

test/lit.cfg.py

139 ↗

(On Diff #140999)

I did not use %llvm-objcopy, I used %objcopy in my test instead. Do I still need that?

[pcc]

test/lit.cfg.py

139 ↗

(On Diff #140999)

Yes because we want to be clear that this is llvm-objcopy and not some other objcopy.

[pcc]

LGTM

tools/gold/gold-plugin.cpp

811–819 ↗

(On Diff #142201)

You should be able to set all of these without the if.

[yunlian]

rebase to trunk.

[yunlian]

Fix an indent error during rebase.

llvm/trunk/include/llvm/LTO/Config.h

struct Config {
 
   /// Setting this field will replace unspecified target triples in input files
   /// with this triple.
   std::string DefaultTriple;
 
   /// Sample PGO profile path.
   std::string SampleProfile;
 
+  /// The directory to store .dwo files.
+  std::string DwoDir;
+
+  /// The objcopy binary used to extract dwo files.
+  std::string Objcopy;
+
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
   /// Whether to emit optimization remarks with hotness informations.
   bool RemarksWithHotness = false;
 
   /// Whether to emit the pass manager debuggging informations.
   bool DebugPassManager = false;

llvm/trunk/lib/LTO/LTO.cpp

#endif
   AddUnsigned(Conf.CGOptLevel);
   AddUnsigned(Conf.CGFileType);
   AddUnsigned(Conf.OptLevel);
   AddUnsigned(Conf.UseNewPM);
   AddString(Conf.OptPipeline);
   AddString(Conf.AAPipeline);
   AddString(Conf.OverrideTriple);
   AddString(Conf.DefaultTriple);
+  AddString(Conf.DwoDir);
 
   // Include the hash for the current module
   auto ModHash = Index.getModuleHash(ModuleID);
   Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
   for (auto F : ExportList)
     // The export list can impact the internalization, be conservative here
     Hasher.update(ArrayRef<uint8_t>((uint8_t *)&F, sizeof(F)));
 

llvm/trunk/lib/LTO/LTOBackend.cpp

 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/Scalar/LoopPassManager.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 #include "llvm/Transforms/Utils/SplitModule.h"
runNewPMCustomPasses(Mod, TM, Conf.OptPipeline, Conf.AAPipeline,
                          Conf.DisableVerify);
   else if (Conf.UseNewPM)
     runNewPMPasses(Conf, Mod, TM, Conf.OptLevel, IsThinLTO);
   else
     runOldPMPasses(Conf, Mod, TM, IsThinLTO, ExportSummary, ImportSummary);
   return !Conf.PostOptModuleHook || Conf.PostOptModuleHook(Task, Mod);
 }
 
+void codegenWithSplitDwarf(Config &Conf, TargetMachine *TM,
+                           AddStreamFn AddStream, unsigned Task, Module &Mod) {
+  SmallString<128> TempFile;
+  int FD = -1;
+  if (auto EC =
+      sys::fs::createTemporaryFile("lto-llvm-fission", "o", FD, TempFile))
+    report_fatal_error("Could not create temporary file " +
+        TempFile.str() + ": " + EC.message());
+  llvm::raw_fd_ostream OS(FD, true);
+  SmallString<1024> DwarfFile(Conf.DwoDir);
+  std::string DwoName = sys::path::filename(Mod.getModuleIdentifier()).str() +
+      "-" + std::to_string(Task) + "-";
+  size_t index = TempFile.str().rfind("lto-llvm-fission");
+  StringRef TempID = TempFile.str().substr(index + 17, 6);
+  DwoName += TempID.str() + ".dwo";
+  sys::path::append(DwarfFile, DwoName);
+  TM->Options.MCOptions.SplitDwarfFile = DwarfFile.str().str();
+
+  legacy::PassManager CodeGenPasses;
+  if (TM->addPassesToEmitFile(CodeGenPasses, OS, Conf.CGFileType))
+    report_fatal_error("Failed to setup codegen");
+  CodeGenPasses.run(Mod);
+
+  if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir))
+    report_fatal_error("Failed to create directory " +
+           Conf.DwoDir + ": " + EC.message());
+
+  SmallVector<const char*, 5> ExtractArgs, StripArgs;
+  ExtractArgs.push_back(Conf.Objcopy.c_str());
+  ExtractArgs.push_back("--extract-dwo");
+  ExtractArgs.push_back(TempFile.c_str());
+  ExtractArgs.push_back(TM->Options.MCOptions.SplitDwarfFile.c_str());
+  ExtractArgs.push_back(nullptr);
+  StripArgs.push_back(Conf.Objcopy.c_str());
+  StripArgs.push_back("--strip-dwo");
+  StripArgs.push_back(TempFile.c_str());
+  StripArgs.push_back(nullptr);
+
+  if (auto Ret = sys::ExecuteAndWait(Conf.Objcopy, ExtractArgs.data())) {
+    report_fatal_error("Failed to extract dwo from " + TempFile.str() +
+        ". Exit code " + std::to_string(Ret));
+  }
+  if (auto Ret = sys::ExecuteAndWait(Conf.Objcopy, StripArgs.data())) {
+    report_fatal_error("Failed to strip dwo from " + TempFile.str() +
+        ". Exit code " + std::to_string(Ret));
+  }
+
+  auto Stream = AddStream(Task);
+  auto Buffer = MemoryBuffer::getFile(TempFile);
+  if (auto EC = Buffer.getError())
+    report_fatal_error("Failed to load file " +
+                       TempFile.str() + ": " + EC.message());
+  *Stream->OS << Buffer.get()->getBuffer();
+  if (auto EC = sys::fs::remove(TempFile))
+    report_fatal_error("Failed to delete file " +
+                       TempFile.str() + ": " + EC.message());
+}
+
 void codegen(Config &Conf, TargetMachine *TM, AddStreamFn AddStream,
              unsigned Task, Module &Mod) {
   if (Conf.PreCodeGenModuleHook && !Conf.PreCodeGenModuleHook(Task, Mod))
     return;
 
+  if (!Conf.DwoDir.empty()) {
+    codegenWithSplitDwarf(Conf, TM, AddStream, Task, Mod);
+    return;
+  }
+
   auto Stream = AddStream(Task);
   legacy::PassManager CodeGenPasses;
   if (TM->addPassesToEmitFile(CodeGenPasses, *Stream->OS, Conf.CGFileType))
     report_fatal_error("Failed to setup codegen");
   CodeGenPasses.run(Mod);
 }
 
 void splitCodeGen(Config &C, TargetMachine *TM, AddStreamFn AddStream,

llvm/trunk/test/lit.cfg.py

tools = [
     ToolSubst('%lli', FindTool('lli'), post='.', extra_args=lli_args),
     ToolSubst('%llc_dwarf', FindTool('llc'), extra_args=llc_args),
     ToolSubst('%go', config.go_executable, unresolved='ignore'),
     ToolSubst('%gold', config.gold_executable, unresolved='ignore'),
     ToolSubst('%ld64', ld64_cmd, unresolved='ignore'),
     ToolSubst('%ocamlc', ocamlc_command, unresolved='ignore'),
     ToolSubst('%ocamlopt', ocamlopt_command, unresolved='ignore'),
     ToolSubst('%opt-viewer', opt_viewer_cmd),
+    ToolSubst('%llvm-objcopy', FindTool('llvm-objcopy')),
 ]
 
 # FIXME: Why do we have both `lli` and `%lli` that do slightly different things?
 tools.extend([
     'dsymutil', 'lli', 'lli-child-target', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer',
     'llvm-config', 'llvm-cov', 'llvm-cxxdump', 'llvm-cvtres', 'llvm-diff', 'llvm-dis',
     'llvm-dwarfdump', 'llvm-extract', 'llvm-isel-fuzzer', 'llvm-opt-fuzzer', 'llvm-lib',
     'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca',

llvm/trunk/test/tools/gold/X86/split-dwarf.ll

+; RUN: rm -rf %t && mkdir -p %t
+; RUN: opt -module-summary %s -o %t/split-dwarf.o
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext  \
+; RUN:    -m elf_x86_64 \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=objcopy=%llvm-objcopy \
+; RUN:    --plugin-opt=dwo_dir=%t/dwo_dir \
+; RUN:    %t/split-dwarf.o --shared -o %t/split-dwarf
+
+; RUN: llvm-dwarfdump -debug-info %t/split-dwarf | FileCheck %s
+; CHECK: DW_AT_GNU_dwo_name{{.*}}dwo_dir/split-dwarf.{{.*}}
+; CHECK-NOT: DW_TAG_subprogram
+; RUN: llvm-dwarfdump -debug-info %t/dwo_dir/split-dwarf.* | FileCheck --check-prefix DWOCHECK %s
+; DWOCHECK: DW_AT_GNU_dwo_name{{.*}}dwo_dir/split-dwarf.o{{.*}}
+; DWOCHECK: DW_AT_name{{.*}}split-dwarf.c
+; DWOCHECK: DW_TAG_subprogram
+
+; RUN:rm -rf %t/dwo_dir
+; RUN: opt  %s -o %t/split-dwarf.o
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext  \
+; RUN:    -m elf_x86_64 \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=objcopy=%llvm-objcopy \
+; RUN:    --plugin-opt=dwo_dir=%t/dwo_dir \
+; RUN:    %t/split-dwarf.o --shared -o %t/split-dwarf
+
+; RUN: llvm-dwarfdump -debug-info %t/split-dwarf | FileCheck --check-prefix LTOCHECK %s
+; LTOCHECK: DW_AT_GNU_dwo_name{{.*}}dwo_dir/ld-temp.{{.*}}
+; LTOCHECK-NOT: DW_TAG_subprogram
+; RUN: llvm-dwarfdump -debug-info %t/dwo_dir/ld-temp.* | FileCheck --check-prefix LTODWOCHECK %s
+; LTODWOCHECK: DW_AT_GNU_dwo_name{{.*}}dwo_dir/ld-temp.o{{.*}}
+; LTODWOCHECK: DW_AT_name{{.*}}split-dwarf.c
+; LTODWOCHECK: DW_TAG_subprogram
+
+; ModuleID = 'split-dwarf.c'
+source_filename = "split-dwarf.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @split_dwarf() #0 !dbg !7 {
+entry:
+  ret i32 0, !dbg !11
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 7.0.0 (https://github.com/llvm-mirror/clang.git b641d31365414ba3ea0305fdaa80369a9efb6bd9) (https://github.com/llvm-mirror/llvm.git 6165a776d1a8bb181be93f2dc97088f7a1abc405)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "split-dwarf.c", directory: "/usr/local/google/home/yunlian/dwp/build/bin")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 7.0.0 (https://github.com/llvm-mirror/clang.git b641d31365414ba3ea0305fdaa80369a9efb6bd9) (https://github.com/llvm-mirror/llvm.git 6165a776d1a8bb181be93f2dc97088f7a1abc405)"}
+!7 = distinct !DISubprogram(name: "split_dwarf", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DILocation(line: 2, column: 2, scope: !7)

llvm/trunk/tools/gold/gold-plugin.cpp

#endif
   // as plugin exclusive to pass to the code generator.
   static std::vector<const char *> extra;
   // Sample profile file path
   static std::string sample_profile;
   // New pass manager
   static bool new_pass_manager = false;
   // Debug new pass manager
   static bool debug_pass_manager = false;
+  // Objcopy for debug fission.
+  static std::string objcopy;
+  // Directory to store the .dwo files.
+  static std::string dwo_dir;
 
   static void process_plugin_option(const char *opt_)
   {
     if (opt_ == nullptr)
       return;
     llvm::StringRef opt = opt_;
 
     if (opt.startswith("mcpu=")) {
static void process_plugin_option(const char *opt_)
     } else if (opt == "disable-verify") {
       DisableVerify = true;
     } else if (opt.startswith("sample-profile=")) {
       sample_profile= opt.substr(strlen("sample-profile="));
     } else if (opt == "new-pass-manager") {
       new_pass_manager = true;
     } else if (opt == "debug-pass-manager") {
       debug_pass_manager = true;
+    } else if (opt.startswith("objcopy=")) {
+      objcopy = opt.substr(strlen("objcopy="));
+    } else if (opt.startswith("dwo_dir=")) {
+      dwo_dir = opt.substr(strlen("dwo_dir="));
     } else {
       // Save this option to pass to the code generator.
       // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
       // add that.
       if (extra.empty())
         extra.push_back("LLVMgold");
 
       extra.push_back(opt_);
case options::OT_SAVE_TEMPS:
     check(Conf.addSaveTemps(output_name + ".",
                             /* UseInputModulePath */ true));
     break;
   }
 
   if (!options::sample_profile.empty())
     Conf.SampleProfile = options::sample_profile;
 
+  Conf.DwoDir = options::dwo_dir;
+
+  Conf.Objcopy = options::objcopy;
+
   // Use new pass manager if set in driver
   Conf.UseNewPM = options::new_pass_manager;
   // Debug new pass manager if requested
   Conf.DebugPassManager = options::debug_pass_manager;
 
   return llvm::make_unique<LTO>(std::move(Conf), Backend,
                                 options::ParallelCodeGenParallelismLevel);
 }