[PATCH] D88400: [llvm-objcopy][MachO] Add support for universal binaries

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/lib/Object/MachOUniversalWriter.cpp:78
 
+Slice::Slice(const Archive &A, uint32_t CPUType, uint32_t CPUSubType,
+             std::string ArchName, uint32_t Align)
----------------
smeenai wrote:
> Is the expectation that this constructor will be used when the `Archive &` is from an existing universal binary, and it's therefore okay to bypass all the checks in the `Slice::create` overload that takes an `Archive &`? If so, we should explicitly comment that (preferably as a Doxygen comment in the header), otherwise people might be confused in the future about when they want to use which function.
yeah, this constructor takes cputype, cpusubtype, and align and doesn't infer them from the archive members (e.g. both lipo/llvm-lipo allow to specify a custom value for the alignment). The main use case is creating a Slice from an archive coming from a universal binary.
I'll add a comment explaining these details.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:396-397
+                                           Buffer &Out) {
+  SmallVector<OwningBinary<Binary>, 1> Binaries;
+  SmallVector<Slice, 1> Slices;
+  for (const auto &O : In.objects()) {
----------------
smeenai wrote:
> I'd up both of these to 2, since I imagine that's the most common universal binary size.
yeah


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88400