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

[Sameer Arora via Phabricator via llvm-commits]

sameerarora101 marked 3 inline comments as done.
sameerarora101 added inline comments.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:29
 
+typedef DenseMap<uint64_t, std::vector<NewArchiveMember>>
+    MembersPerArchitectureMap;
----------------
smeenai wrote:
> 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.
Got it, thanks!


================
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));
 
----------------
smeenai wrote:
> 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?
oh wow, great idea. It actually helps me compute `std::tie(ArchCPUType, ArchCPUSubtype)` just once as well (while creating the config). Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85334