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

Fri Aug 14 10:04:25 PDT 2020

smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM

================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:123

+static uint64_t getCPUID(uint32_t CPUType, uint32_t CPUSubtype) {
+  switch (CPUType) {
----------------
sameerarora101 wrote:
> smeenai wrote:
> > sameerarora101 wrote:
> > > jhenderson wrote:
> > > > Looks to me like this function is only partially tested? What about the other three CPUs and the default case?
> > > Ok also added test for `CPU_TYPE_ARM64`,  `CPU_TYPE_X86_64` and the default case. `CPU_TYPE_ARM64_32` does not have a subtype so I didn't add a test for it.
> > Can you have two files with a CPUType that falls under the default case but different CPUSubtypes, and confirm that they get put in the same slice of the output file?
> Besides the 4 non-default cases below, the file `llvm/lib/Object/MachOObjectFile.cpp` supports the following CPU types (which will fall under default case):
> - `MachO::CPU_TYPE_I386`
> - `MachO::CPU_TYPE_POWERPC`
> - `MachO::CPU_TYPE_POWERPC64`
> 
> However, for all these default CPU types, there is just a **single** subtype:
> - `MachO::CPU_SUBTYPE_I386_ALL`
> - `MachO::CPU_SUBTYPE_POWERPC_ALL`
> - `MachO::CPU_SUBTYPE_POWERPC_ALL`
> 
> Thus, I am unable to have two files having the same CPU type that falls under default case but have different Subtypes :( 
> Any other way I should test this?
Got it, yeah, I don't think there's anything we can do then.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85334