[PATCH] D85334: [llvm-libtool-darwin] Support universal outputs

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:29
 
+typedef DenseMap<uint64_t, std::vector<NewArchiveMember>>
+    MembersPerArchitectureMap;
----------------
Note that DenseMap doesn't guarantee deterministic iteration order, which is something to watch out for in general (you need std::map or MapVector to get determinism). Over here though, you sort the slices afterwards, so it's fine.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:139-140
   uint32_t ArchCPUType, ArchCPUSubtype;
   std::tie(ArchCPUType, ArchCPUSubtype) = MachO::getCPUTypeFromArchitecture(
       MachO::getArchitectureFromName(ArchType));
 
----------------
Not this diff, but it's not ideal to be recomputing this for every single input file.

Since you have to do this computation in `createStaticLibrary` as well now, perhaps you could make `ArchCPUType` and `ArchCPUSubtype` part of your `Config`, and then pass that into this function?


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:381
+    }
+    Expected<SmallVector<Slice, 2>> Slices = buildSlices(OutputBinaries);
+    if (!Slices)
----------------
Nit: newline above this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85334