[PATCH] D136380: [lld-macho] Map file should map symbols to their original bitcode file

[Roger Kim via Phabricator via llvm-commits]

Roger accepted this revision.
Roger added a comment.
This revision is now accepted and ready to land.

Looks good !



================
Comment at: lld/MachO/MapFile.cpp:45
+struct MapInfo {
+  SmallVector<InputFile *> files;
+  SmallVector<Defined *> liveSymbols;
----------------
Could we make this name more specific? Like "SymbolEmittingFiles" or "EmittingFilese"?


================
Comment at: lld/MachO/MapFile.cpp:50
+
+MapInfo gatherMapInfo() {
+  MapInfo info;
----------------
can this function be made static?


================
Comment at: lld/test/MachO/map-file.ll:5-6
+;; Verify that we map symbols to their original bitcode input files, not the
+;; intermediate object files. Also verify that we don't emit redundant object
+;; files if no symbol needs to reference them.
+
----------------
For my benefit, could you explain more what this sentence means? Why would a "redundant object file" be and how would they be emitted?


================
Comment at: lld/test/MachO/map-file.ll:11
+
+; RUN: %lld -dylib %t/foo.thinlto.o %t/bar.thinlto.o -o %t/foobar.thinlto -map \
+; RUN:   %t/foobar.thinlto.map
----------------
As I understand, this flag makes the output into a dynamic library. Is this feature specific to dynamic libraries?


================
Comment at: lld/test/MachO/map-file.ll:24-48
+; FOOBAR:      # Path: {{.*}}{{/|\\}}map-file.ll.tmp/foobar.thinlto
+; FOOBAR-NEXT: # Arch: x86_64
+; FOOBAR-NEXT: # Object files:
+; FOOBAR-NEXT: [  0] linker synthesized
+; FOOBAR-NEXT: [  1] {{.*}}{{/|\\}}map-file.ll.tmp/foo.thinlto.o
+; FOOBAR-NEXT: [  2] {{.*}}{{/|\\}}map-file.ll.tmp/bar.thinlto.o
+; FOOBAR-NEXT: # Sections:
----------------
If I understand correctly, these two tests are there to show that the maybe_weak symbol gets attributed to the bar file regardless of the order of the input files (because maybe_weak symbol is not weak in the bar file). Could we write that down as a comment here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136380