[PATCH] D62753: [llvm-lipo] Implement -archs

[Michael Trent via Phabricator via llvm-commits]

mtrent requested changes to this revision.
mtrent added a comment.
This revision now requires changes to proceed.

I did not download or run this code, but I believe it does not correctly divine the Arch flag from a given Mach-O binary. This is straight-forward to fix.



================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:209
+  // cctools lipo.
+  Triple::ArchType Arch = MachOObjectFile::getArch(CPUType);
+  if (Arch == Triple::ArchType::UnknownArch)
----------------
The problem here is that there are Mach-O Arch types that are identifiable by cputype and cpusubtype, such as "x86_64h" and "armv7k". The general Triple API does not seem to support this; so Mach-O tools get this from the MachOObjectFile or from the MachOObjectFile::getArchTriple. 

Since the code calling printArchFromCPUType has a MachOObjectFile (Obj), I would simply pass that MachOObjectFile in, ask it for its Triple (Obj->getArch()), and then handle an unknown triple. But you could remove the dependency on a MachOObjectFile by replacing your call to static MachOObjectFile::getArch() with a call to static MachOObjectFile::getArchTriple(): given the cputype and cpusubtype it will return the ArchFlag for you. But that's kind of backwards ... 


================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:224
+    for (const MachOUniversalBinary::ObjectForArch &Obj : UO->objects())
+      printArchFromCPUType(Obj.getCPUType(), Obj.getCPUSubType());
+  else if (auto O =
----------------
See my note above; if it were me I'd pass Obj in here, and maybe rename the "printArchFromCPUType" to "printArchOrUnknown". 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62753