[PATCH] D103488: [lld/mac] Emit only one LC_LOAD_DYLIB per dylib

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/MachO/Writer.cpp:681
   int64_t dylibOrdinal = 1;
+  llvm::DenseMap<llvm::StringRef, int64_t> ordinalForInstallName;
   for (InputFile *file : inputFiles) {
----------------
nit: no need for llvm::


================
Comment at: lld/MachO/Writer.cpp:716
+      //   in Foo.framework/Versions/A/Foo.tbd, while the explicit link will
+      //   usually find Foo.framwork/Foo.tdb. These are usually two identical
+      //   but distinct files (concrete example: CFNetwork.framework, reexported
----------------



================
Comment at: lld/test/MachO/dylink-ordinal.s:3
+
+# --no-leading-lines needed for .tbd files.
+# RUN: rm -rf %t; split-file --no-leading-lines %s %t
----------------
also, TIL about this flag... looks like I can clean up some of our tests :D


================
Comment at: lld/test/MachO/dylink-ordinal.s:6-23
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/main.s -o %t/main.o
+# RUN: %lld -o %t/main -L%t -lFoo -lBar -lSystem %t/main.o
+# RUN: llvm-objdump --lazy-bind -d --no-show-raw-insn %t/main | FileCheck %s
+
+# CHECK: callq 0x[[#%x,FOO_OFF:]]
+# CHECK-NEXT: callq 0x[[#%x,BAR_OFF:]]
+
----------------
are these really necessary? All these pass in the current implementation (only the `LOAD` check below fails)... plus they seem pretty similar to what's already in dylink.s


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

https://reviews.llvm.org/D103488