Handle ARMv6KZ naming

[tyomitch]

Summary

• ARMv6KZ is the "canonical" name, given in the ARMARM
• ARMv6Z is an "official abbreviation" for it, mentioned in the ARMARM
• ARMv6ZK is a popular misspelling, which we should support as an alias.

The patch corrects the handling of the names.

Functional changes:

• ARMv6Z no longer treated as an architecture in its own right
• ARMv6ZK renamed to ARMv6KZ, accepting ARMv6ZK as an alias
• arm1176jz-s and arm1176jzf-s recognized as ARMv6ZK, instead of ARMv6K
• default ARMv6K CPU changed to arm1176j-s

[rengolin]

Same comment as the other similar review: what's the motivation behind this?

I don't want to break legacy systems, I don't know what legacy systems there are out there using these names, and I don't want to find out by breaking their stuff and getting a bug report 6 months from now when all will be forgotten.

I don't think having those extra names has such a big compile-time impact to warrant removal without a clear reason.

[tyomitch]

In D14568#287420, @rengolin wrote:

Same comment as the other similar review: what's the motivation behind this?

I don't want to break legacy systems, I don't know what legacy systems there are out there using these names, and I don't want to find out by breaking their stuff and getting a bug report 6 months from now when all will be forgotten.

I don't think having those extra names has such a big compile-time impact to warrant removal without a clear reason.

Same as for the other patch, all of these names will continue to be accepted as aliases, but only the ARMARM-endorsed would be used throughout the source code.

[rengolin]

unittests/ADT/TripleTest.cpp

863

This example came from somewhere I can't remember... Maybe FreeBSD? Or RPi?

[tyomitch]

unittests/ADT/TripleTest.cpp

863

This comes from your patch from half a year ago (r237797) which changed the triple being tested from "armv6-unknown-freebsd" to "armv6k-unknown-eabi".

I suspect you were thus recording the existing state of the target parser, rather than an intention behind it?

I'm not sure it makes a lot of sense to have an ARMv6KZ core the default CPU for both ARMv6K and ARMv6KZ.
If one wants to target ARMv6KZ, they would specify ARMv6KZ as the target, wouldn't they?

[rengolin]

LGTM, thanks!

unittests/ADT/TripleTest.cpp

863

If one wants to target ARMv6KZ, they would specify ARMv6KZ as the target, wouldn't they?

The problem here is how systems *do* reference those architectures and not how they *should*.

However, I'm willing to sacrifice legacy in name of clarity. In the long run, it's probably cheaper to deal with a few backslashes than carry legacy forever for no apparent reason.

[tyomitch]

Rebased over r253198

include/llvm/Support/ARMTargetParser.def

 ARM_ARCH("armv5tej", AK_ARMV5TEJ, "5TEJ", "v5e", ARMBuildAttrs::CPUArch::v5TEJ,
           FK_NONE, AEK_DSP)
 ARM_ARCH("armv6", AK_ARMV6, "6", "v6", ARMBuildAttrs::CPUArch::v6,
           FK_VFPV2, AEK_DSP)
 ARM_ARCH("armv6k", AK_ARMV6K, "6K", "v6k", ARMBuildAttrs::CPUArch::v6K,
           FK_VFPV2, AEK_DSP)
 ARM_ARCH("armv6t2", AK_ARMV6T2, "6T2", "v6t2", ARMBuildAttrs::CPUArch::v6T2,
           FK_NONE, AEK_DSP)
-ARM_ARCH("armv6z", AK_ARMV6Z, "6Z", "v6z", ARMBuildAttrs::CPUArch::v6KZ,
-          FK_VFPV2, (AEK_SEC | AEK_DSP))
-ARM_ARCH("armv6zk", AK_ARMV6ZK, "6ZK", "v6zk", ARMBuildAttrs::CPUArch::v6KZ,
+ARM_ARCH("armv6kz", AK_ARMV6KZ, "6KZ", "v6kz", ARMBuildAttrs::CPUArch::v6KZ,
           FK_VFPV2, (AEK_SEC | AEK_DSP))
 ARM_ARCH("armv6-m", AK_ARMV6M, "6-M", "v6m", ARMBuildAttrs::CPUArch::v6_M,
           FK_NONE, AEK_NONE)
 ARM_ARCH("armv7-a", AK_ARMV7A, "7-A", "v7", ARMBuildAttrs::CPUArch::v7,
           FK_NEON, AEK_DSP)
 ARM_ARCH("armv7-r", AK_ARMV7R, "7-R", "v7r", ARMBuildAttrs::CPUArch::v7,
           FK_NONE, (AEK_HWDIV | AEK_DSP))
 ARM_ARCH("armv7-m", AK_ARMV7M, "7-M", "v7m", ARMBuildAttrs::CPUArch::v7,

 ARM_CPU_NAME("arm946e-s", AK_ARMV5TE, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("arm966e-s", AK_ARMV5TE, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("arm968e-s", AK_ARMV5TE, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("arm10e", AK_ARMV5TE, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("arm1020e", AK_ARMV5TE, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("arm1022e", AK_ARMV5TE, FK_NONE, true, AEK_NONE)
 ARM_CPU_NAME("arm926ej-s", AK_ARMV5TEJ, FK_NONE, true, AEK_NONE)
 ARM_CPU_NAME("arm1136jf-s", AK_ARMV6, FK_VFPV2, true, AEK_NONE)
-ARM_CPU_NAME("arm1176j-s", AK_ARMV6K, FK_NONE, false, AEK_NONE)
-ARM_CPU_NAME("arm1176jz-s", AK_ARMV6K, FK_NONE, false, AEK_NONE)
+ARM_CPU_NAME("arm1176j-s", AK_ARMV6K, FK_NONE, true, AEK_NONE)
+ARM_CPU_NAME("arm1176jz-s", AK_ARMV6KZ, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("mpcore", AK_ARMV6K, FK_VFPV2, false, AEK_NONE)
 ARM_CPU_NAME("mpcorenovfp", AK_ARMV6K, FK_NONE, false, AEK_NONE)
-ARM_CPU_NAME("arm1176jzf-s", AK_ARMV6K, FK_VFPV2, true, AEK_NONE)
-ARM_CPU_NAME("arm1176jzf-s", AK_ARMV6Z, FK_VFPV2, true, AEK_NONE)
-ARM_CPU_NAME("arm1176jzf-s", AK_ARMV6ZK, FK_VFPV2, true, AEK_NONE)
+ARM_CPU_NAME("arm1176jzf-s", AK_ARMV6KZ, FK_VFPV2, true, AEK_NONE)
 ARM_CPU_NAME("arm1156t2-s", AK_ARMV6T2, FK_NONE, true, AEK_NONE)
 ARM_CPU_NAME("arm1156t2f-s", AK_ARMV6T2, FK_VFPV2, false, AEK_NONE)
 ARM_CPU_NAME("cortex-m0", AK_ARMV6M, FK_NONE, true, AEK_NONE)
 ARM_CPU_NAME("cortex-m0plus", AK_ARMV6M, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("cortex-m1", AK_ARMV6M, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("sc000", AK_ARMV6M, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("cortex-a5", AK_ARMV7A, FK_NEON_VFPV4, false, (AEK_SEC | AEK_MP))
 ARM_CPU_NAME("cortex-a7", AK_ARMV7A, FK_NEON_VFPV4, false,

lib/Support/TargetParser.cpp

 }
 
 static StringRef getArchSynonym(StringRef Arch) {
   return StringSwitch<StringRef>(Arch)
       .Case("v5", "v5t")
       .Case("v5e", "v5te")
       .Case("v6hl", "v6k")
       .Cases("v6m", "v6sm", "v6s-m", "v6-m")
+      .Cases("v6z", "v6zk", "v6kz")
       .Cases("v7", "v7a", "v7hl", "v7l", "v7-a")
       .Case("v7r", "v7-r")
       .Case("v7m", "v7-m")
       .Case("v7em", "v7e-m")
       .Cases("v8", "v8a", "aarch64", "arm64", "v8-a")
       .Case("v8.1a", "v8.1-a")
       .Default(Arch);
 }
unsigned llvm::ARM::parseArchVersion(StringRef Arch) {
   case ARM::AK_IWMMXT2:
   case ARM::AK_XSCALE:
   case ARM::AK_ARMV5TEJ:
     return 5;
   case ARM::AK_ARMV6:
   case ARM::AK_ARMV6J:
   case ARM::AK_ARMV6K:
   case ARM::AK_ARMV6T2:
-  case ARM::AK_ARMV6Z:
-  case ARM::AK_ARMV6ZK:
+  case ARM::AK_ARMV6KZ:
   case ARM::AK_ARMV6M:
     return 6;
   case ARM::AK_ARMV7A:
   case ARM::AK_ARMV7R:
   case ARM::AK_ARMV7M:
   case ARM::AK_ARMV7S:
   case ARM::AK_ARMV7EM:
   case ARM::AK_ARMV7K:
     return 7;
   case ARM::AK_ARMV8A:
   case ARM::AK_ARMV8_1A:
     return 8;
   }
   return 0;
 }

lib/Support/Triple.cpp

static Triple::SubArchType parseSubArch(StringRef SubArchName) {
   case ARM::AK_ARMV5TE:
   case ARM::AK_IWMMXT:
   case ARM::AK_IWMMXT2:
   case ARM::AK_XSCALE:
   case ARM::AK_ARMV5TEJ:
     return Triple::ARMSubArch_v5te;
   case ARM::AK_ARMV6:
   case ARM::AK_ARMV6J:
-  case ARM::AK_ARMV6Z:
     return Triple::ARMSubArch_v6;
   case ARM::AK_ARMV6K:
-  case ARM::AK_ARMV6ZK:
+  case ARM::AK_ARMV6KZ:
     return Triple::ARMSubArch_v6k;
   case ARM::AK_ARMV6T2:
     return Triple::ARMSubArch_v6t2;
   case ARM::AK_ARMV6M:
     return Triple::ARMSubArch_v6m;
   case ARM::AK_ARMV7A:
   case ARM::AK_ARMV7R:
     return Triple::ARMSubArch_v7;

lib/Target/ARM/ARM.td

 
 def ARMv6     : Architecture<"armv6",     "ARMv6",    [HasV6Ops]>;
 
 def ARMv6t2   : Architecture<"armv6t2",   "ARMv6t2",  [HasV6T2Ops,
                                                        FeatureDSP]>;
 
 def ARMv6k    : Architecture<"armv6k",    "ARMv6k",   [HasV6KOps]>;
 
-def ARMv6z    : Architecture<"armv6z",    "ARMv6z",   [HasV6KOps,
+def ARMv6kz   : Architecture<"armv6kz",   "ARMv6kz",  [HasV6KOps,
                                                        FeatureTrustZone]>;
 
 def ARMv6m    : Architecture<"armv6-m",   "ARMv6m",   [HasV6MOps,
                                                        FeatureNoARM,
                                                        FeatureDB,
                                                        FeatureMClass]>;
 
 def ARMv6sm   : Architecture<"armv6s-m",  "ARMv6sm",  [HasV6MOps,
def : Processor<"arm1136jf-s",      ARMV6Itineraries,   [ARMv6,
                                                          FeatureVFP2,
                                                          FeatureHasSlowFPVMLx]>;
 
 def : Processor<"cortex-m0",        ARMV6Itineraries,   [ARMv6m]>;
 def : Processor<"cortex-m0plus",    ARMV6Itineraries,   [ARMv6m]>;
 def : Processor<"cortex-m1",        ARMV6Itineraries,   [ARMv6m]>;
 def : Processor<"sc000",            ARMV6Itineraries,   [ARMv6m]>;
 
-def : Processor<"arm1176jz-s",      ARMV6Itineraries,   [ARMv6z]>;
-def : Processor<"arm1176jzf-s",     ARMV6Itineraries,   [ARMv6z,
+def : Processor<"arm1176jz-s",      ARMV6Itineraries,   [ARMv6kz]>;
+def : Processor<"arm1176jzf-s",     ARMV6Itineraries,   [ARMv6kz,
                                                          FeatureVFP2,
                                                          FeatureHasSlowFPVMLx]>;
 
 def : Processor<"mpcorenovfp",      ARMV6Itineraries,   [ARMv6k]>;
 def : Processor<"mpcore",           ARMV6Itineraries,   [ARMv6k,
                                                          FeatureVFP2,
                                                          FeatureHasSlowFPVMLx]>;
 

lib/Target/ARM/ARMSubtarget.h

enum ARMProcFamilyEnum {
     CortexA17, CortexR4, CortexR4F, CortexR5, CortexR7, CortexA53, CortexA57,
     CortexA72, Krait, Swift
   };
   enum ARMProcClassEnum {
     None, AClass, RClass, MClass
   };
   enum ARMArchEnum {
     ARMv2, ARMv2a, ARMv3, ARMv3m, ARMv4, ARMv4t, ARMv5, ARMv5t, ARMv5te,
-    ARMv5tej, ARMv6, ARMv6k, ARMv6z, ARMv6t2, ARMv6m, ARMv6sm, ARMv7a, ARMv7r,
+    ARMv5tej, ARMv6, ARMv6k, ARMv6kz, ARMv6t2, ARMv6m, ARMv6sm, ARMv7a, ARMv7r,
     ARMv7m, ARMv7em, ARMv8a, ARMv81a
   };
 
   /// ARMProcFamily - ARM processor family: Cortex-A8, Cortex-A9, and others.
   ARMProcFamilyEnum ARMProcFamily;
 
   /// ARMProcClass - ARM processor class: None, AClass, RClass or MClass.
   ARMProcClassEnum ARMProcClass;

lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp

case ARM::AK_ARMV6J:
     break;
 
   case ARM::AK_ARMV6T2:
     setAttributeItem(ARM_ISA_use, Allowed, false);
     setAttributeItem(THUMB_ISA_use, AllowThumb32, false);
     break;
 
   case ARM::AK_ARMV6K:
-  case ARM::AK_ARMV6Z:
-  case ARM::AK_ARMV6ZK:
+  case ARM::AK_ARMV6KZ:
     setAttributeItem(ARM_ISA_use, Allowed, false);
     setAttributeItem(THUMB_ISA_use, Allowed, false);
     setAttributeItem(Virtualization_use, AllowTZ, false);
     break;
 
   case ARM::AK_ARMV6M:
     setAttributeItem(THUMB_ISA_use, Allowed, false);
     break;

test/MC/ARM/directive-arch-armv6z.s

 @ Test the .arch directive for armv6z
 
 @ This test case will check the default .ARM.attributes value for the
 @ armv6z architecture.
 
 @ RUN: llvm-mc -triple arm-eabi -filetype asm %s \
 @ RUN:   | FileCheck %s -check-prefix CHECK-ASM
 @ RUN: llvm-mc -triple arm-eabi -filetype obj %s \
 @ RUN:   | llvm-readobj -arm-attributes | FileCheck %s -check-prefix CHECK-ATTR
 
   .syntax unified
   .arch armv6z
 
-@ CHECK-ASM:  .arch armv6z
+@ CHECK-ASM:  .arch armv6kz
 
 @ CHECK-ATTR: FileAttributes {
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: CPU_name
-@ CHECK-ATTR:     Value: 6Z
+@ CHECK-ATTR:     Value: 6KZ
 @ CHECK-ATTR:   }
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: CPU_arch
 @ CHECK-ATTR:     Description: ARM v6KZ
 @ CHECK-ATTR:   }
 @ CHECK-ATTR:   Attribute {
 @ CHECK-ATTR:     TagName: ARM_ISA_use
 @ CHECK-ATTR:     Description: Permitted

test/MC/ARM/directive-arch-armv6zk.s

-@ Test the .arch directive for armv6zk
-
-@ This test case will check the default .ARM.attributes value for the
-@ armv6zk architecture.
-
-@ RUN: llvm-mc -triple arm-eabi -filetype asm %s \
-@ RUN:   | FileCheck %s -check-prefix CHECK-ASM
-@ RUN: llvm-mc -triple arm-eabi -filetype obj %s \
-@ RUN:   | llvm-readobj -arm-attributes | FileCheck %s -check-prefix CHECK-ATTR
-
-  .syntax unified
-  .arch armv6zk
-
-@ CHECK-ASM:  .arch armv6zk
-
-@ CHECK-ATTR: FileAttributes {
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: CPU_name
-@ CHECK-ATTR:     Value: 6ZK
-@ CHECK-ATTR:   }
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: CPU_arch
-@ CHECK-ATTR:     Description: ARM v6KZ
-@ CHECK-ATTR:   }
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: ARM_ISA_use
-@ CHECK-ATTR:     Description: Permitted
-@ CHECK-ATTR:   }
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: THUMB_ISA_use
-@ CHECK-ATTR:     Description: Thumb-1
-@ CHECK-ATTR:   }
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: Virtualization_use
-@ CHECK-ATTR:     Description: TrustZone
-@ CHECK-ATTR:   }
-@ CHECK-ATTR: }
-

unittests/ADT/TripleTest.cpp

// Standard ARM Architectures.
     EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv6j-unknown-eabi");
     EXPECT_EQ("arm1136j-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv6k-unknown-eabi");
+    EXPECT_EQ("arm1176j-s", Triple.getARMCPUForArch());
+  }
+  {
+    llvm::Triple Triple("armv6kz-unknown-eabi");
     EXPECT_EQ("arm1176jzf-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv6zk-unknown-eabi");
     EXPECT_EQ("arm1176jzf-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv6t2-unknown-eabi");
     EXPECT_EQ("arm1156t2-s", Triple.getARMCPUForArch());