[ARM] Add support for armv7ve triple in llvm (PR31358).

[manojgupta]

Summary

Gcc supports target armv7ve which is armv7-a with virtualization
extensions. This change add support for this in llvm for gcc
compatibility.

Also remove redundant FeatureHWDiv, FeatureHWDivARM for a few models
as this is specified automatically by FeatureVirtualization.

The corresponding clang change is at https://reviews.llvm.org/D29773 .

Subscribers: aemerson, llvm-commits, rengolin

Differential Revision: https://reviews.llvm.org/D29472

[manojgupta]

Adding reviewers from Arm.

[ab]

I don't think we can unilaterally add MachO subtypes: I would remove all MachO changes.

Otherwise, this seems reasonable. Can you update the TargetParser unittest?

[manojgupta]

Updates as requested:

1. Removed Mach-o changes.
2. Updated TargetParser UnitTest.

[richard.barton.arm]

Hi Manoj

Virtualization requires TrustZone so I have added a few comments where I think this is missing. Looks like there are existing bugs in this area which you have propagated into your patch for ARMv7-VE. Would be good to fix the whole lot at once.

Shouldn't new TargetParser options also get a clang test for the new commandline? I was also expecting a build attributes test added to test/CodeGen/ARM/build-attributes.ll testing the Virtualization and Div attributes are set correctly.

include/llvm/Support/ARMTargetParser.def

86

The Virtualization extension requires TrustZone, so I would expect to see AEK_SEC in this list, unless this is implied by AEK_VIRT. I guess from the Divide macros also given that it is not.

lib/Target/ARM/ARM.td

450

Same comment here about lack of TrustZone with Virtualization. I think this should reallybe part of the FeatureVirtualization definition higher in the file. This definition also includes the two divide Features, so they are redundant here and can be removed. The same comment could perhaps be made for a number of other targets in this file.

lib/Target/ARM/ARMSubtarget.h

55

Being super pedantic, after ARMv7-A would be a more logical order. Super super pedantic there

[manojgupta]

Hi Richard,

After making the changes you suggested, I am getting a test fail in build-attributes for cortex-r52 wrt Tag_Virtualization_use.
I noticed that cortex-r52 uses armv8-r arch model but armv8-r does not have TrustZone in its supported features. Is this correct ?
And if so, I can not add TrustZone to FeatureVirtualization sublist.

If TrustZone is indeed supported for Armv8-r, I'll simply fix the test for cortex-r52.

def : ProcessorModel<"cortex-r52", CortexR52Model, [ARMv8r, ProcR52,

FeatureFPAO]>;

def ARMv8r : Architecture<"armv8-r", "ARMv8r", [HasV8Ops,

FeatureRClass,
FeatureDB,
FeatureHWDiv,
FeatureHWDivARM,
FeatureT2XtPk,
FeatureDSP,
FeatureCRC,
FeatureMP,
FeatureVirtualization,  // No Trustzone in this list
FeatureFPARMv8,
FeatureNEON]>;

Failure details:

Before (ToT):
.eabi_attribute 68, 2 @ Tag_Virtualization_use

After adding TrustZone:
.eabi_attribute 68, 3 @ Tag_Virtualization_use

Thanks,
Manoj

[manojgupta]

Updated as per Richard comments. Did not add TrustZone to FeatureVirtualization since Armv8-r supports virtualization but not TrustZone.
Added more tests for armv7ve.

[richard.barton.arm]

Hi Manoj

Sorry for not getting back to you yesterday. As you have concluded, ARMv8-R has not got the Security extension, which ruins my idea of neatly encapsulating this in the Virtualization extension definiton.

The new patch is fine save for a missing multiprocessing extension feature in the .td

lib/Target/ARM/ARM.td

427

FeatureMP is missing from here. The Virtualization extension on ARMv7-A requires the MP extension, Security Extension and Divide extensions (and some other things which are not interesting to MC).

[manojgupta]

Added FeatureMP.

[richard.barton.arm]

Thanks Manoj - this now also LGTM.

[manojgupta]

Thanks Richard. I will wait for Mehdi's approval since he is a blocking reviewer.

[efriedma]

You can ignore that; I added Medhi because the initial version of the patch had some MachO changes that would have needed approval from someone at Apple.

[mehdi_amini]

Yeah ignore me please :)

include/llvm/ADT/Triple.h

     ARMSubArch_v7m,
     ARMSubArch_v7s,
     ARMSubArch_v7k,
+    ARMSubArch_v7ve,
     ARMSubArch_v6,
     ARMSubArch_v6m,
     ARMSubArch_v6k,

include/llvm/Support/ARMTargetParser.def

           FK_NONE, ARM::AEK_HWDIV)
 ARM_ARCH("armv7e-m", AK_ARMV7EM, "7E-M", "v7em", ARMBuildAttrs::CPUArch::v7E_M,
           FK_NONE, (ARM::AEK_HWDIV | ARM::AEK_DSP))
+ARM_ARCH("armv7ve", AK_ARMV7VE, "7VE", "v7ve", ARMBuildAttrs::CPUArch::v7,
+          FK_NEON, (ARM::AEK_VIRT | ARM::AEK_HWDIVARM | ARM::AEK_HWDIV |
+          ARM::AEK_DSP))
 ARM_ARCH("armv8-a", AK_ARMV8A, "8-A", "v8", ARMBuildAttrs::CPUArch::v8_A,
          FK_CRYPTO_NEON_FP_ARMV8,
          (ARM::AEK_SEC | ARM::AEK_MP | ARM::AEK_VIRT | ARM::AEK_HWDIVARM |

lib/Support/TargetParser.cpp

   case ARM::AK_ARMV8R:
     return ARM::PK_R;
   case ARM::AK_ARMV7A:
+  case ARM::AK_ARMV7VE:
   case ARM::AK_ARMV7K:
   case ARM::AK_ARMV8A:
   case ARM::AK_ARMV8_1A:

   case ARM::AK_ARMV7S:
   case ARM::AK_ARMV7EM:
   case ARM::AK_ARMV7K:
+  case ARM::AK_ARMV7VE:
     return 7;
   case ARM::AK_ARMV8A:
   case ARM::AK_ARMV8_1A:

lib/Support/Triple.cpp

     return Triple::ARMSubArch_v7s;
   case ARM::AK_ARMV7EM:
     return Triple::ARMSubArch_v7em;
+  case ARM::AK_ARMV7VE:
+    return Triple::ARMSubArch_v7ve;
   case ARM::AK_ARMV8A:
     return Triple::ARMSubArch_v8;
   case ARM::AK_ARMV8_1A:

lib/Target/ARM/ARM.td

                                                        FeatureDSP,
                                                        FeatureT2XtPk]>;
 
+def ARMv7ve   : Architecture<"armv7ve",   "ARMv7ve",  [HasV7Ops,
+                                                       FeatureNEON,
+                                                       FeatureDB,
+                                                       FeatureDSP,
+                                                       FeatureHWDiv,
+                                                       FeatureHWDivARM,
+                                                       FeatureVirtualization,
+                                                       FeatureAClass,
+                                                       FeatureT2XtPk]>;
+
 def ARMv8a    : Architecture<"armv8-a",   "ARMv8a",   [HasV8Ops,
                                                        FeatureAClass,
                                                        FeatureDB,

lib/Target/ARM/ARMSubtarget.h

   enum ARMArchEnum {
     ARMv2, ARMv2a, ARMv3, ARMv3m, ARMv4, ARMv4t, ARMv5, ARMv5t, ARMv5te,
     ARMv5tej, ARMv6, ARMv6k, ARMv6kz, ARMv6t2, ARMv6m, ARMv6sm, ARMv7a, ARMv7r,
-    ARMv7m, ARMv7em, ARMv8a, ARMv81a, ARMv82a, ARMv8mMainline, ARMv8mBaseline,
-    ARMv8r
+    ARMv7m, ARMv7em, ARMv7ve, ARMv8a, ARMv81a, ARMv82a, ARMv8mMainline,
+    ARMv8mBaseline, ARMv8r
   };
 
 public:

test/CodeGen/ARM/div.ll

 ; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-HWDIV
 ; RUN: llc < %s -mtriple=arm-none-eabi -mcpu=cortex-a8    | \
 ; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=armv7ve-apple-ios                | \
+; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-HWDIV
+; RUN: llc < %s -mtriple=thumbv7ve-apple-ios              | \
+; RUN:     FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-HWDIV \
+; RUN:                  -check-prefix=CHECK-THUMB
 
 define i32 @f1(i32 %a, i32 %b) {
 entry:
 ; CHECK-LABEL: f1
 ; CHECK-SWDIV: __divsi3
 
+; CHECK-THUMB: .thumb_func
 ; CHECK-HWDIV: sdiv
 
 ; CHECK-EABI: __aeabi_idiv

 ; CHECK-LABEL: f2
 ; CHECK-SWDIV: __udivsi3
 
+; CHECK-THUMB: .thumb_func
 ; CHECK-HWDIV: udiv
 
 ; CHECK-EABI: __aeabi_uidiv

 ; CHECK-LABEL: f3
 ; CHECK-SWDIV: __modsi3
 
+; CHECK-THUMB: .thumb_func
 ; CHECK-HWDIV: sdiv
 ; CHECK-HWDIV: mls
 

 ; CHECK-LABEL: f4
 ; CHECK-SWDIV: __umodsi3
 
+; CHECK-THUMB: .thumb_func
 ; CHECK-HWDIV: udiv
 ; CHECK-HWDIV: mls
 

unittests/Support/TargetParserTest.cpp

 
 namespace {
 const char *ARMArch[] = {
-    "armv2",        "armv2a",      "armv3",        "armv3m",      "armv4",
-    "armv4t",       "armv5",       "armv5t",       "armv5e",      "armv5te",
-    "armv5tej",     "armv6",       "armv6j",       "armv6k",      "armv6hl",
-    "armv6t2",      "armv6kz",     "armv6z",       "armv6zk",     "armv6-m",
-    "armv6m",       "armv6sm",     "armv6s-m",     "armv7-a",     "armv7",
-    "armv7a",       "armv7hl",     "armv7l",       "armv7-r",     "armv7r",
-    "armv7-m",      "armv7m",      "armv7k",       "armv7s",      "armv7e-m",
-    "armv7em",      "armv8-a",     "armv8",        "armv8a",      "armv8.1-a",
-    "armv8.1a",     "armv8.2-a",   "armv8.2a",     "armv8-r",     "armv8r",
-    "armv8-m.base", "armv8m.base", "armv8-m.main", "armv8m.main", "iwmmxt",
-    "iwmmxt2",      "xscale"};
+    "armv2",     "armv2a",       "armv3",       "armv3m",       "armv4",
+    "armv4t",    "armv5",        "armv5t",      "armv5e",       "armv5te",
+    "armv5tej",  "armv6",        "armv6j",      "armv6k",       "armv6hl",
+    "armv6t2",   "armv6kz",      "armv6z",      "armv6zk",      "armv6-m",
+    "armv6m",    "armv6sm",      "armv6s-m",    "armv7-a",      "armv7",
+    "armv7a",    "armv7ve",      "armv7hl",     "armv7l",       "armv7-r",
+    "armv7r",    "armv7-m",      "armv7m",      "armv7k",       "armv7s",
+    "armv7e-m",  "armv7em",      "armv8-a",     "armv8",        "armv8a",
+    "armv8.1-a", "armv8.1a",     "armv8.2-a",   "armv8.2a",     "armv8-r",
+    "armv8r",    "armv8-m.base", "armv8m.base", "armv8-m.main", "armv8m.main",
+    "iwmmxt",    "iwmmxt2",      "xscale"};
 
 bool testARMCPU(StringRef CPUName, StringRef ExpectedArch,
                 StringRef ExpectedFPU, unsigned ExpectedFlags,

   EXPECT_TRUE(
       testARMArch("armv7k", "generic", "v7k",
                           ARMBuildAttrs::CPUArch::v7));
+  EXPECT_TRUE(
+      testARMArch("armv7ve", "generic", "v7ve",
+                          ARMBuildAttrs::CPUArch::v7));
 }
 
 bool testARMExtension(StringRef CPUName, unsigned ArchKind, StringRef ArchExt) {

 
 TEST(TargetParserTest, ARMparseArchEndianAndISA) {
   const char *Arch[] = {
-      "v2",    "v2a",    "v3",    "v3m",  "v4",   "v4t",  "v5",    "v5t",
-      "v5e",   "v5te",   "v5tej", "v6",   "v6j",  "v6k",  "v6hl",  "v6t2",
-      "v6kz",  "v6z",    "v6zk",  "v6-m", "v6m",  "v6sm", "v6s-m", "v7-a",
-      "v7",    "v7a",    "v7hl",  "v7l",  "v7-r", "v7r",  "v7-m",  "v7m",
-      "v7k",   "v7s",    "v7e-m", "v7em", "v8-a", "v8",   "v8a",   "v8.1-a",
-      "v8.1a", "v8.2-a", "v8.2a", "v8-r"};
+      "v2",     "v2a",   "v3",     "v3m",   "v4",   "v4t",  "v5",    "v5t",
+      "v5e",    "v5te",  "v5tej",  "v6",    "v6j",  "v6k",  "v6hl",  "v6t2",
+      "v6kz",   "v6z",   "v6zk",   "v6-m",  "v6m",  "v6sm", "v6s-m", "v7-a",
+      "v7",     "v7a",   "v7ve",   "v7hl",  "v7l",  "v7-r", "v7r",   "v7-m",
+      "v7m",    "v7k",   "v7s",    "v7e-m", "v7em", "v8-a", "v8",    "v8a",
+      "v8.1-a", "v8.1a", "v8.2-a", "v8.2a", "v8-r"};
 
   for (unsigned i = 0; i < array_lengthof(Arch); i++) {
     std::string arm_1 = "armeb" + (std::string)(Arch[i]);

       EXPECT_EQ(ARM::PK_R, ARM::parseArchProfile(ARMArch[i]));
       continue;
     case ARM::AK_ARMV7A:
+    case ARM::AK_ARMV7VE:
     case ARM::AK_ARMV7K:
     case ARM::AK_ARMV8A:
     case ARM::AK_ARMV8_1A: