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

[Sameer Arora via Phabricator via llvm-commits]

sameerarora101 added inline comments.


================
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:
> sameerarora101 wrote:
> > sameerarora101 wrote:
> > > 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?
> > Added diff for constant V8 here: D85041
> I think your other patch is fine. It may be beneficial to look at the history of the file you're modifiying to see what else got changed when other subtypes were added.
Yup, thanks. Furthermore, I'll also wait on D85041 to get landed so that I can make modifications here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84770