[ARM] Add Marvell PJ4 cpu

[Kai]

Summary

The Marvell PJ4 cpus are ARMv7 compliant cpus similar to Cortex-A8 but based on a custom design.

This patch adds basic support for the PJ4 cpus. Details can be found in the Linux kernel README https://www.kernel.org/doc/readme/Documentation-arm-Marvell-README.

[rengolin]

Hi Kai,

This request actually has two different ideas:

1. Adding the host CPU detection (not target's), so that when running Clang natively on the Armada, you detect the right CPU. Returning "marvel-pj4" and implementing the core's defaults in the TargetParser (no NEON, MMX2, etc) should be enough for your purposes.

2. Implementing PJ4's model in TableGen, which is used for scheduler and code-generation purposes, not compliance. This is a more serious issue and needs some long experimentation to find the correct options. I don't think this is what you want.

So, the question is: What's the motivation behind this?

We generally only add new cores when we have better scheduling or performance reasons to do so, and you're not adding any of that. The detection system only works when running native, for everything else, we need the specific triples and arch options to get it right.

You'll also need tests, to make sure the options are chosen correctly, and to express your intent to future investigations from this commit. I don't know how to test the cpuinfo parsing, but once you have the cpu name, you can use the TargetParserTest unit test to make sure you have the right options.

cheers,
--renato

[Kai]

Hi Renato,

sorry for the very long delay. I am still interested in this. My motivation is to detect the host cpu in order to use the right features. I updated the code and also added tests. Further I removed the TableGen code for the cpu model.

Regards,
Kai

[rengolin]

Kai,

Below are some inline reviews, but overall, it's looking good. Please update to the current trunk and check the GCC behaviour on your kits to see if they do the right thing (and what arch names they use), so we don't deviate from the norm.

thanks,
--renato

include/llvm/Support/ARMTargetParser.def

227

Move this declaration down below Non-standard Arch names..

228

According to the kernel link, only PJ4B-MP / PJ4C have ideva, while all have idivt, so I'd not add the support for AEK_HWDIVARM here. Maybe we should add two flavours, 'marvell-pj4' and 'marvell-pj4c'? What does GCC do?

This is the first arch that we have publicly that supports AEK_IWMMXT2, we should make use of that. :)

lib/Support/Host.cpp

255

This looks a bit dangerous to rely on. Basically, if this is Marvell and the CPU part below is correct, we should be good to go. I'd remove the cpu arch check altogether. See Qualcomm's implementation on recent trunk.

265

Right, these guys are different in the idiva implementation, and maybe we should support both of them as separate.

[Kai]

Hi Renato,
I updated the patch according to your comments.

A current gcc provides only a -mcpu=marvell-pj4 switch. There is no differentiation between the PJ4 and the PJ4C. gcc defines the cpu as plain armv7-a with no special instructions enabled, but defines a special cost model. (See https://github.com/gcc-mirror/gcc/blob/master/gcc/config/arm/arm-cpus.in#L1377 and https://github.com/gcc-mirror/gcc/blob/master/gcc/config/arm/arm-cpus.in#L467.)

The only drawback I currently see is that the cpu is detected but cannot specified on the command line with -mcpu=marvell-pj4. This feels a bit weird. Does the gcc cost model provide enough information to translate this to LLVM?

include/llvm/Support/ARMTargetParser.def

               ARM::AEK_HWDIVTHUMB))
 ARM_CPU_NAME("krait", ARMV7A, FK_NEON_VFPV4, false,
              (ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB))
+ARM_CPU_NAME("marvell-pj4", ARMV7A, FK_VFPV3, false,
+             (ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB))
 ARM_CPU_NAME("cortex-r4", ARMV7R, FK_NONE, true, ARM::AEK_NONE)
 ARM_CPU_NAME("cortex-r4f", ARMV7R, FK_VFPV3_D16, false, ARM::AEK_NONE)
 ARM_CPU_NAME("cortex-r5", ARMV7R, FK_VFPV3_D16, false,

lib/Support/Host.cpp

     }
   }
 
+  if (Implementer == "0x56") { // Marvell Technology Group Ltd.
+    // Look for the CPU architecture line.
+    for (unsigned I = 0, E = Lines.size(); I != E; ++I)
+      if (Lines[I].startswith("CPU architecture"))
+        // Architecture must be ARMv7.
+        if (Lines[I].substr(16).ltrim("\t :") != "7")
+          return "generic";
+    // Look for the CPU part line.
+    for (unsigned I = 0, E = Lines.size(); I != E; ++I)
+      if (Lines[I].startswith("CPU part"))
+        // The CPU part is a 3 digit hexadecimal number with a 0x prefix. The
+        // values correspond to the "Part number" in the CP15/c0 register. The
+        // contents are specified in the various processor manuals.
+        return StringSwitch<const char *>(Lines[I].substr(8).ltrim("\t :"))
+            .Case("0x581", "marvell-pj4") // Sheeva PJ4 / PJ4B
+            .Case("0x584", "marvell-pj4") // Sheeva PJ4B-MP / PJ4C
+            .Default("generic");
+  }
+
   return "generic";
 }
 

unittests/Support/Host.cpp

   EXPECT_EQ(sys::detail::getHostCPUNameForARM("CPU implementer : 0x51\n"
                                               "CPU part        : 0x06f"),
             "krait");
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM("CPU implementer : 0x56\n"
+                                              "CPU architecture: 7\n"
+                                              "CPU part        : 0x581"),
+            "marvell-pj4");
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM("CPU implementer : 0x56\n"
+                                              "CPU architecture: 7\n"
+                                              "CPU part        : 0x584"),
+            "marvell-pj4");
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM("CPU implementer : 0x56\n"
+                                              "CPU architecture: 6\n"
+                                              "CPU part        : 0x584"),
+            "generic");
 }
 
 TEST(getLinuxHostCPUName, AArch64) {

unittests/Support/TargetParserTest.cpp

   EXPECT_TRUE(testARMCPU("krait", "armv7-a", "neon-vfpv4",
                          ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
                          "7-A"));
+  EXPECT_TRUE(testARMCPU("marvell-pj4", "armv7-a", "vfpv3",
+                         ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP,
+                         "7-A"));
   EXPECT_TRUE(testARMCPU("cortex-r4", "armv7-r", "none",
                          ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP, "7-R"));
   EXPECT_TRUE(testARMCPU("cortex-r4f", "armv7-r", "vfpv3-d16",

                          "7-S"));
 }
 
-static constexpr unsigned NumARMCPUArchs = 82;
+static constexpr unsigned NumARMCPUArchs = 83;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector<StringRef, NumARMCPUArchs> List;