[PATCH] D84770: [llvm-libtool-darwin] Add support for -arch_only

[Sameer Arora via Phabricator via llvm-commits]

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


================
Comment at: llvm/docs/CommandGuide/llvm-libtool-darwin.rst:64-67
+.. option:: -arch_only <architecture>
+
+ Build a static library only for the specified `<architecture>` and ignore all
+ other architectures in the files.
----------------
jhenderson wrote:
> I should have been paying more attention to this before, but it would be good to have the options listed in alphabetical order, I think.
Ok, I have ordered them alphabetically now.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:126-128
+  // Currently LLVM doesn't have the constant MachO::CPU_SUBTYPE_ARM64_V8.
+  // When the constant is added, following modification needs to be done for the
+  // case of MachO::CPU_TYPE_ARM64 below:
----------------
jhenderson wrote:
> Rather than a comment that will be forgotten about and/or ignored, could you not just add the constant?
Is it as simple as adding a constant? Nothing else needs to be done?

Here is a potential place where I think the constant should go (under MachO.h):
```
enum CPUSubTypeARM {
  CPU_SUBTYPE_ARM_ALL = 0,
  CPU_SUBTYPE_ARM_V4T = 5,
  CPU_SUBTYPE_ARM_V6 = 6,
  CPU_SUBTYPE_ARM_V5 = 7,
  CPU_SUBTYPE_ARM_V5TEJ = 7,
  CPU_SUBTYPE_ARM_XSCALE = 8,
  CPU_SUBTYPE_ARM_V7 = 9,
  //  unused  ARM_V7F     = 10,
  CPU_SUBTYPE_ARM_V7S = 11,
  CPU_SUBTYPE_ARM_V7K = 12,
  CPU_SUBTYPE_ARM_V6M = 14,
  CPU_SUBTYPE_ARM_V7M = 15,
  CPU_SUBTYPE_ARM_V7EM = 16
};

enum CPUSubTypeARM64 {
  CPU_SUBTYPE_ARM64_ALL = 0,
  CPU_SUBTYPE_ARM64E = 2,
};

enum CPUSubTypeARM64_32 { CPU_SUBTYPE_ARM64_32_V8 = 1 };
```
However, I am also unsure of the RHS value that needs to be assigned to the constant. Any ideas?


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:249
+    if (ArchiveOrError) {
+      consumeError(MachOObjOrErr.takeError());
+      if (Error E = processArchive(Members, **ArchiveOrError, FileName, C))
----------------
jhenderson wrote:
> Why are you throwing away this error rahter than reporting it earlier?
So a universal member can either be a `MachOObjectFile` or an `Archive` - so we need to try casting the member in both ways. First I try casting it as an object file. If that gives an error, it might be the case that the member is an Archive. Now, if I succeed on casting the member as an Archive, then I can throw away the error that I received from casting the member as an object file as the member was an archive and should not have been casted as an object file in first place.

`llvm-lipo` also seems to be following this design (under `printBinaryArchs`):
```
for (const auto &O : UO->objects()) {
      Expected<std::unique_ptr<MachOObjectFile>> MachOObjOrError =
          O.getAsObjectFile();
      if (MachOObjOrError) {
        OS << Slice(MachOObjOrError->get()).getArchString() << " ";
        continue;
      }
      Expected<std::unique_ptr<Archive>> ArchiveOrError = O.getAsArchive();
      if (ArchiveOrError) {
        consumeError(MachOObjOrError.takeError());
        OS << Slice(ArchiveOrError->get()).getArchString() << " ";
        continue;
      }
      consumeError(ArchiveOrError.takeError());
      reportError(Binary->getFileName(), MachOObjOrError.takeError());
    }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84770