[ARM] Reject invalid -march values

[john.brawn]

Summary

Restructure Triple::getARMCPUForArch so that invalid values will return nullptr, while retaining the behaviour that an argument specifying no particular architecture version will give a default CPU. This will be used by clang (in D9602) to give an error on invalid -march values.

Also restructure the extraction of the architecture version from the MArch string a little to hopefully make what it's doing clearer.

[rengolin]

Hi John,

I just happened to have a look at this method yesterday, due to D9435, and I plan on using it later on. For now, this looks a lot better than what we have.

LGTM, thanks!

lib/Support/Triple.cpp

case llvm::Triple::NetBSD:
     break;
   case llvm::Triple::Win32:
     // FIXME: this is invalid for WindowsCE
     return "cortex-a9";
   default:
     break;
   }
 
+  // MArch is expected to be of the form (arm|thumb)?(eb)?(v.+)?(eb)?
+  // Only the (v.+) part is relevant for determining the CPU, as it determines
+  // the architecture version, so we first remove the surrounding parts.
+  // (ep9312|iwmmxt|xscale)(eb)? is also permitted, so we have to be a bit
+  // careful when removing the leading (arm|thumb)?(eb)? as we don't want to
+  // permit things like armep9312.
   const char *result = nullptr;
   size_t offset = StringRef::npos;
   if (MArch.startswith("arm"))
     offset = 3;
-  if (MArch.startswith("thumb"))
+  else if (MArch.startswith("thumb"))
     offset = 5;
   if (offset != StringRef::npos && MArch.substr(offset, 2) == "eb")
     offset += 2;
-  if (MArch.endswith("eb"))
+  else if (MArch.endswith("eb"))
     MArch = MArch.substr(0, MArch.size() - 2);
-  if (offset != StringRef::npos)
-    result = llvm::StringSwitch<const char *>(MArch.substr(offset))
+  if (offset != StringRef::npos && (offset == MArch.size() || MArch[offset] == 'v'))
+    MArch = MArch.substr(offset);
+
+  if (MArch == "") {
+    // If no specific architecture version is requested, return the minimum CPU
+    // required by the OS and environment.
+    switch (getOS()) {
+    case llvm::Triple::NetBSD:
+      switch (getEnvironment()) {
+      case llvm::Triple::GNUEABIHF:
+      case llvm::Triple::GNUEABI:
+      case llvm::Triple::EABIHF:
+      case llvm::Triple::EABI:
+        return "arm926ej-s";
+      default:
+        return "strongarm";
+      }
+    case llvm::Triple::NaCl:
+      return "cortex-a8";
+    default:
+      switch (getEnvironment()) {
+      case llvm::Triple::EABIHF:
+      case llvm::Triple::GNUEABIHF:
+        return "arm1176jzf-s";
+      default:
+        return "arm7tdmi";
+      }
+    }
+  } else {
+    result = llvm::StringSwitch<const char *>(MArch)
       .Cases("v2", "v2a", "arm2")
       .Case("v3", "arm6")
       .Case("v3m", "arm7m")
       .Case("v4", "strongarm")
       .Case("v4t", "arm7tdmi")
       .Cases("v5", "v5t", "arm10tdmi")
       .Cases("v5e", "v5te", "arm1022e")
       .Case("v5tej", "arm926ej-s")
       .Case("v6", "arm1136jf-s")
       .Case("v6j", "arm1136j-s")
       .Cases("v6k", "v6z", "v6zk", "arm1176jzf-s")
       .Case("v6t2", "arm1156t2-s")
       .Cases("v6m", "v6-m", "v6sm", "v6s-m", "cortex-m0")
       .Cases("v7", "v7a", "v7-a", "v7l", "v7-l", "cortex-a8")
       .Cases("v7s", "v7-s", "swift")
       .Cases("v7r", "v7-r", "cortex-r4")
       .Cases("v7m", "v7-m", "cortex-m3")
       .Cases("v7em", "v7e-m", "cortex-m4")
       .Cases("v8", "v8a", "v8-a", "cortex-a53")
       .Cases("v8.1a", "v8.1-a", "generic")
-      .Default(nullptr);
-  else
-    result = llvm::StringSwitch<const char *>(MArch)
       .Case("ep9312", "ep9312")
       .Case("iwmmxt", "iwmmxt")
       .Case("xscale", "xscale")
       .Default(nullptr);
+  }
 
-  if (result)
-    return result;
-
-  // If all else failed, return the most base CPU with thumb interworking
-  // supported by LLVM.
-  // FIXME: Should warn once that we're falling back.
-  switch (getOS()) {
-  case llvm::Triple::NetBSD:
-    switch (getEnvironment()) {
-    case llvm::Triple::GNUEABIHF:
-    case llvm::Triple::GNUEABI:
-    case llvm::Triple::EABIHF:
-    case llvm::Triple::EABI:
-      return "arm926ej-s";
-    default:
-      return "strongarm";
-    }
-  case llvm::Triple::NaCl:
-    return "cortex-a8";
-  default:
-    switch (getEnvironment()) {
-    case llvm::Triple::EABIHF:
-    case llvm::Triple::GNUEABIHF:
-      return "arm1176jzf-s";
-    default:
-      return "arm7tdmi";
-    }
-  }
 }

unittests/ADT/TripleTest.cpp

TEST(TripleTest, getARMCPUForArch) {
     llvm::Triple Triple("armv7-apple-ios7");
     EXPECT_STREQ("cortex-a8", Triple.getARMCPUForArch());
     EXPECT_STREQ("swift", Triple.getARMCPUForArch("armv7s"));
   }
   {
     llvm::Triple Triple("arm--nacl");
     EXPECT_STREQ("cortex-a8", Triple.getARMCPUForArch("arm"));
   }
+  // armebv6 and armv6eb are permitted, but armebv6eb is not
+  {
+    llvm::Triple Triple("armebv6-non-eabi");
+    EXPECT_STREQ("arm1136jf-s", Triple.getARMCPUForArch());
+  }
+  {
+    llvm::Triple Triple("armv6eb-none-eabi");
+    EXPECT_STREQ("arm1136jf-s", Triple.getARMCPUForArch());
+  }
+  {
+    llvm::Triple Triple("armebv6eb-none-eabi");
+    EXPECT_EQ(nullptr, Triple.getARMCPUForArch());
+  }
+  // armeb is permitted, but armebeb is not
+  {
+    llvm::Triple Triple("armeb-none-eabi");
+    EXPECT_STREQ("arm7tdmi", Triple.getARMCPUForArch());
+  }
+  {
+    llvm::Triple Triple("armebeb-none-eabi");
+    EXPECT_EQ(nullptr, Triple.getARMCPUForArch());
+  }
+  // xscaleeb is permitted, but armebxscale is not
+  {
+    llvm::Triple Triple("xscaleeb-none-eabi");
+    EXPECT_STREQ("xscale", Triple.getARMCPUForArch());
+  }
+  {
+    llvm::Triple Triple("armebxscale-none-eabi");
+    EXPECT_EQ(nullptr, Triple.getARMCPUForArch());
+  }
 }
 
 TEST(TripleTest, NormalizeARM) {
   EXPECT_EQ("armv6--netbsd-eabi", Triple::normalize("armv6-netbsd-eabi"));
   EXPECT_EQ("armv7--netbsd-eabi", Triple::normalize("armv7-netbsd-eabi"));
   EXPECT_EQ("armv6eb--netbsd-eabi", Triple::normalize("armv6eb-netbsd-eabi"));
   EXPECT_EQ("armv7eb--netbsd-eabi", Triple::normalize("armv7eb-netbsd-eabi"));
   EXPECT_EQ("armv6--netbsd-eabihf", Triple::normalize("armv6-netbsd-eabihf"));