Handle ARMv6-J as an alias, instead of fake architecture

[tyomitch]

Summary

This follows D14577 to treat ARMv6-J as an alias for ARMv6,
instead of an architecture in its own right.

The functional change is that the default CPU when targeting ARMv6-J
changes from arm1136j-s to arm1136jf-s, which is currently used as
the default CPU for ARMv6; both are, in fact, ARMv6-J CPUs.

The J-bit (Jazelle support) is irrelevant to LLVM, and it doesn't
affect code generation, attributes, optimizations, or anything else,
apart from selecting the default CPU.

[rengolin]

I'm ok with the change, but I'd rather hear from @joerg, @compnerd, @t.p.northover, @jroelofs to make sure we're not breaking anything.

cheers,
--renato

unittests/ADT/TripleTest.cpp

855 ↗

(On Diff #40430)

Non-functional triples were recorded because of the default CPU they had more than what they actually meant. In this case, the difference is between having VFP or not, which makes a huge difference on soft vs hard float and could wreak havoc on build systems.

I'm not saying that it's a good idea to keep the unknown legacy, but we gotta at least give some thought to this change.

[tyomitch]

unittests/ADT/TripleTest.cpp

855 ↗

(On Diff #40430)

Wow, so the difference between armv6j and armv6 triples is not Jazelle but VFP?

Couldn't be any more obscure than this...

[jroelofs]

GCC seems to think arm1136j-s and arm1136jz-s are real things. FWIW, it treats arm1136j-s as v6J + no VFP2, arm1136jf-s as v6J + VFP2, arm1136jz-s as v6zk + no VFP2, and arm1136jz-s as v6zk + VFP2. That being said, it's possible GCC was wrong for adding them in the first place. I have no idea how many people rely on this behavior in gcc, or in clang.

Perhaps it's best to "break" this with an error message that says: "x is not supported, did you mean y?"... that way users whose build systems are dependent on this mcpu at least get an explanation on what they have to fix, rather than them complaining that something just doesn't work anymore.

[jroelofs]

In D14755#293154, @jroelofs wrote:

GCC seems to think arm1136j-s and arm1136jz-s are real things. FWIW, it treats arm1136j-s as v6J + no VFP2, arm1136jf-s as v6J + VFP2, arm1136jz-s as v6zk + no VFP2, and arm1136jz-s as v6zk + VFP2. That being said, it's possible GCC was wrong for adding them in the first place. I have no idea how many people rely on this behavior in gcc, or in clang.

Perhaps it's best to "break" this with an error message that says: "x is not supported, did you mean y?"... that way users whose build systems are dependent on this mcpu at least get an explanation on what they have to fix, rather than them complaining that something just doesn't work anymore.

erm... ignore this ^, I didn't read the thread carefully enough.

[tyomitch]

Jonathan, do you perhaps know if GCC treats ARMv6 and ARMv6J as the same thing or different things?

[jroelofs]

They're treated the same AFAICT.

[tyomitch]

In D14755#293188, @jroelofs wrote:

They're treated the same AFAICT.

Would it, then, be acceptable to unify them in LLVM too, which is what this patch does?

[jroelofs]

In D14755#293750, @tyomitch wrote:

In D14755#293188, @jroelofs wrote:

They're treated the same AFAICT.

Would it, then, be acceptable to unify them in LLVM too, which is what this patch does?

Yeah, that's my understanding of all of this.

[meadori]

It is my understanding as well that -march=armv6 and -march=armv6j are treated the same code gen wise by GCC. However, they do cause different builtin macro to be defined. For example:

#define ARM_ARCH_6J 1

and:

#define ARM_ARCH_6 1

I don't know what the current behavior of Clang/LLVM is with respect to these macros, whether this change to unify them will impact that, or if anyone out there has code that will break due to the macros, but it is something to consider (I would probably err on the side of caution, but maybe that is too conservative).

llvm/trunk/include/llvm/Support/ARMTargetParser.def

ARM_ARCH("armv8.1-a", AK_ARMV8_1A, "8.1-A", "v8.1a", ARMBuildAttrs::CPUArch::v8,
                              AEK_HWDIV | AEK_DSP | AEK_CRC))
 // Non-standard Arch names.
 ARM_ARCH("iwmmxt", AK_IWMMXT, "iwmmxt", "", ARMBuildAttrs::CPUArch::v5TE,
           FK_NONE, AEK_NONE)
 ARM_ARCH("iwmmxt2", AK_IWMMXT2, "iwmmxt2", "", ARMBuildAttrs::CPUArch::v5TE,
           FK_NONE, AEK_NONE)
 ARM_ARCH("xscale", AK_XSCALE, "xscale", "", ARMBuildAttrs::CPUArch::v5TE,
           FK_NONE, AEK_NONE)
-ARM_ARCH("armv6j", AK_ARMV6J, "6J", "v6", ARMBuildAttrs::CPUArch::v6,
-          FK_NONE, AEK_DSP)
 ARM_ARCH("armv7s", AK_ARMV7S, "7-S", "v7s", ARMBuildAttrs::CPUArch::v7,
           FK_NEON_VFPV4, AEK_DSP)
 ARM_ARCH("armv7k", AK_ARMV7K, "7-K", "v7k", ARMBuildAttrs::CPUArch::v7,
           FK_NONE, AEK_DSP)
 #undef ARM_ARCH
 
 #ifndef ARM_ARCH_EXT_NAME
 #define ARM_ARCH_EXT_NAME(NAME, ID, FEATURE, NEGFEATURE)

 ARM_CPU_NAME("arm9e", AK_ARMV5TE, FK_NONE, false, AEK_NONE)
 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("arm1136j-s", AK_ARMV6, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("arm1136jf-s", AK_ARMV6, FK_VFPV2, true, AEK_NONE)
+ARM_CPU_NAME("arm1136jz-s", AK_ARMV6, 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_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-m7", AK_ARMV7EM, FK_FPV5_D16, false, AEK_NONE)
 ARM_CPU_NAME("cortex-a53", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, true, AEK_CRC)
 ARM_CPU_NAME("cortex-a57", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false, AEK_CRC)
 ARM_CPU_NAME("cortex-a72", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false, AEK_CRC)
 ARM_CPU_NAME("cyclone", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false, AEK_CRC)
 // Non-standard Arch names.
 ARM_CPU_NAME("iwmmxt", AK_IWMMXT, FK_NONE, true, AEK_NONE)
 ARM_CPU_NAME("xscale", AK_XSCALE, FK_NONE, true, AEK_NONE)
-ARM_CPU_NAME("arm1136j-s", AK_ARMV6J, FK_NONE, true, AEK_NONE)
-ARM_CPU_NAME("arm1136jz-s", AK_ARMV6J, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("swift", AK_ARMV7S, FK_NEON_VFPV4, true,
              (AEK_HWDIVARM | AEK_HWDIV))
 ARM_CPU_NAME("cortex-a7", AK_ARMV7K, FK_NONE, true, AEK_HWDIVARM | AEK_HWDIV)
 // Invalid CPU
 ARM_CPU_NAME("invalid", AK_INVALID, FK_INVALID, true, AEK_INVALID)
 #undef ARM_CPU_NAME

llvm/trunk/lib/Support/TargetParser.cpp

return StringSwitch<StringRef>(FPU)
       .Case("neon-vfpv3", "neon")
       .Default(FPU);
 }
 
 static StringRef getArchSynonym(StringRef Arch) {
   return StringSwitch<StringRef>(Arch)
       .Case("v5", "v5t")
       .Case("v5e", "v5te")
+      .Case("v6j", "v6")
       .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")
unsigned llvm::ARM::parseArchVersion(StringRef Arch) {
   case ARM::AK_ARMV5T:
   case ARM::AK_ARMV5TE:
   case ARM::AK_IWMMXT:
   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_ARMV6KZ:
   case ARM::AK_ARMV6M:
     return 6;
   case ARM::AK_ARMV7A:
   case ARM::AK_ARMV7R:
   case ARM::AK_ARMV7M:

llvm/trunk/lib/Support/Triple.cpp

case ARM::AK_ARMV5T:
     return Triple::ARMSubArch_v5;
   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:
     return Triple::ARMSubArch_v6;
   case ARM::AK_ARMV6K:
   case ARM::AK_ARMV6KZ:
     return Triple::ARMSubArch_v6k;
   case ARM::AK_ARMV6T2:
     return Triple::ARMSubArch_v6t2;
   case ARM::AK_ARMV6M:
     return Triple::ARMSubArch_v6m;

llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp

void ARMTargetELFStreamer::emitArchDefaultAttributes() {
   case ARM::AK_ARMV4:
     setAttributeItem(ARM_ISA_use, Allowed, false);
     break;
 
   case ARM::AK_ARMV4T:
   case ARM::AK_ARMV5T:
   case ARM::AK_ARMV5TE:
   case ARM::AK_ARMV6:
-  case ARM::AK_ARMV6J:
     setAttributeItem(ARM_ISA_use, Allowed, false);
     setAttributeItem(THUMB_ISA_use, Allowed, false);
     break;
 
   case ARM::AK_ARMV6T2:
     setAttributeItem(ARM_ISA_use, Allowed, false);
     setAttributeItem(THUMB_ISA_use, AllowThumb32, false);
     break;

llvm/trunk/test/MC/ARM/directive-arch-armv6j.s

-@ Test the .arch directive for armv6j
-
-@ This test case will check the default .ARM.attributes value for the
-@ armv6j 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 armv6j
-
-@ CHECK-ASM:  .arch armv6j
-
-@ CHECK-ATTR: FileAttributes {
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: CPU_name
-@ CHECK-ATTR:     Value: 6J
-@ CHECK-ATTR:   }
-@ CHECK-ATTR:   Attribute {
-@ CHECK-ATTR:     TagName: CPU_arch
-@ CHECK-ATTR:     Description: ARM v6
-@ 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: }
-

llvm/trunk/unittests/ADT/TripleTest.cpp

// Standard ARM Architectures.
     EXPECT_EQ("arm926ej-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv6-unknown-eabi");
     EXPECT_EQ("arm1136jf-s", Triple.getARMCPUForArch());
   }
   {
     llvm::Triple Triple("armv6j-unknown-eabi");
-    EXPECT_EQ("arm1136j-s", Triple.getARMCPUForArch());
+    EXPECT_EQ("arm1136jf-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());