[ARM] Restructure cpu handling in the driver to mostly use the triple

[john.brawn]

Summary

Using the target cpu to determine some behaviour is sprinkled in several places in the driver, but in almost all the information that is needed can be found in the triple. Restructure things so that the triple is used, and the cpu is only used if the exact cpu name is needed.

Also add a check that the -mcpu argument is valid, and correct the -march argument checking so that it handles -march=native correctly. I would have liked to move these checks into the computation of the triple, but the triple is calculated several times in several places and that would lead to multiple error messages for the same thing.

[john.brawn]

Fix typo.

[rengolin]

Hi John,

I'm about to commit some changes to ARMTargetParser that may affect your patch. I have tested both LLVM and Clang with it and found no bugs, but that doesn't guarantee much. I've also added a lot more unit tests to the Triple interface, which should stabilise things.

I was going to refactor this part later, but it seems that your changes are mostly orthogonal to what I planned. I'd welcome if you could rebase your changes after I send my patch, to make sure they still work as you'd expect.

cheers,
--renato

[rengolin]

Hi John,

The changes look fine with two comments. Don't worry too much if ARMTargetParser is a bit confusing, once we got all arch / cpu changes into it, I'll refactor it to make more sense. For now, I'm trying hard to not change the behaviour in any way.

cheers,
--renato

lib/Driver/Tools.cpp

620 ↗

(On Diff #26151)

These two should either go into Triple or ARMTargetParser. I'll do that move later, for now, they can stay here. Just add a comment:

// FIXME: Move to ARMTargetParser.

780 ↗

(On Diff #26151)

Not any more. Check ARMTargetParser::getCanonicalArch() and parseArch().

[t.p.northover]

I've checked the iOS/Darwin behaviour and it seems reasonable from that perspective too.

Tim.

[john.brawn]

Add a couple of FIXME comments, and use ARMTargetParser::getCanonicalArchName to decide if the -march argument is valid. Doing that means moving the handling of -march=native from getARMCPUForMArch to getARMArch, which I think should be OK.

[rengolin]

With the extra comment, LGTM.

Thanks!
--renato

lib/Driver/Tools.cpp

5741 ↗

(On Diff #26220)

FIXME: Move to ARMTargetParser.

cfe/trunk/lib/Driver/Tools.cpp

case llvm::ARM::FK_NEON_VFPV4:
     Features.push_back("+neon");
     Features.push_back("+vfp4");
     break;
   default:
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
   }
 }
 
+// FIXME: Move to ARMTargetParser.
+static int getARMSubArchVersionNumber(const llvm::Triple &Triple) {
+  switch (Triple.getSubArch()) {
+  case llvm::Triple::ARMSubArch_v8_1a:
+  case llvm::Triple::ARMSubArch_v8:
+    return 8;
+  case llvm::Triple::ARMSubArch_v7:
+  case llvm::Triple::ARMSubArch_v7em:
+  case llvm::Triple::ARMSubArch_v7m:
+  case llvm::Triple::ARMSubArch_v7s:
+    return 7;
+  case llvm::Triple::ARMSubArch_v6:
+  case llvm::Triple::ARMSubArch_v6m:
+  case llvm::Triple::ARMSubArch_v6k:
+  case llvm::Triple::ARMSubArch_v6t2:
+    return 6;
+  case llvm::Triple::ARMSubArch_v5:
+  case llvm::Triple::ARMSubArch_v5te:
+    return 5;
+  case llvm::Triple::ARMSubArch_v4t:
+    return 4;
+  default:
+    return 0;
+  }
+}
+
+// FIXME: Move to ARMTargetParser.
+static bool isARMMProfile(const llvm::Triple &Triple) {
+  switch (Triple.getSubArch()) {
+  case llvm::Triple::ARMSubArch_v7em:
+  case llvm::Triple::ARMSubArch_v7m:
+  case llvm::Triple::ARMSubArch_v6m:
+    return true;
+  default:
+    return false;
+  }
+}
+
 // Select the float ABI as determined by -msoft-float, -mhard-float, and
 // -mfloat-abi=.
 StringRef tools::arm::getARMFloatABI(const Driver &D, const ArgList &Args,
                                      const llvm::Triple &Triple) {
   StringRef FloatABI;
   if (Arg *A = Args.getLastArg(options::OPT_msoft_float,
                                options::OPT_mhard_float,
                                options::OPT_mfloat_abi_EQ)) {
StringRef tools::arm::getARMFloatABI(const Driver &D, const ArgList &Args,
   // If unspecified, choose the default based on the platform.
   if (FloatABI.empty()) {
     switch (Triple.getOS()) {
     case llvm::Triple::Darwin:
     case llvm::Triple::MacOSX:
     case llvm::Triple::IOS: {
       // Darwin defaults to "softfp" for v6 and v7.
       //
-      // FIXME: Factor out an ARM class so we can cache the arch somewhere.
-      std::string ArchName =
-        arm::getLLVMArchSuffixForARM(arm::getARMTargetCPU(Args, Triple),
-                                     arm::getARMArch(Args, Triple));
-      if (StringRef(ArchName).startswith("v6") ||
-          StringRef(ArchName).startswith("v7"))
+      if (getARMSubArchVersionNumber(Triple) == 6 ||
+          getARMSubArchVersionNumber(Triple) == 7)
         FloatABI = "softfp";
       else
         FloatABI = "soft";
       break;
     }
 
     // FIXME: this is invalid for WindowsCE
     case llvm::Triple::Win32:
default:
       case llvm::Triple::EABIHF:
         FloatABI = "hard";
         break;
       case llvm::Triple::EABI:
         // EABI is always AAPCS, and if it was not marked 'hard', it's softfp
         FloatABI = "softfp";
         break;
       case llvm::Triple::Android: {
-        std::string ArchName =
-          arm::getLLVMArchSuffixForARM(arm::getARMTargetCPU(Args, Triple),
-                                       arm::getARMArch(Args, Triple));
-        if (StringRef(ArchName).startswith("v7"))
+        if (getARMSubArchVersionNumber(Triple) == 7)
           FloatABI = "softfp";
         else
           FloatABI = "soft";
         break;
       }
       default:
         // Assume "soft", but warn the user we are guessing.
         FloatABI = "soft";
static void getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   }
 
   // Honor -mfpu=.
   if (const Arg *A = Args.getLastArg(options::OPT_mfpu_EQ))
     getARMFPUFeatures(D, A, Args, Features);
   if (const Arg *A = Args.getLastArg(options::OPT_mhwdiv_EQ))
     getARMHWDivFeatures(D, A, Args, Features);
 
-  // -march is handled in getARMCPUForMarch by translating it into a CPU name,
-  // but it needs to return an empty string on invalid arguments. We therefore
-  // check and give an error here if the -march is invalid.
-  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
-    if (!Triple.getARMCPUForArch(A->getValue()))
+  // Check if -march is valid by checking if it can be canonicalised. getARMArch
+  // is used here instead of just checking the -march value in order to handle
+  // -march=native correctly.
+  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
+    StringRef Arch = arm::getARMArch(Args, Triple);
+    if (llvm::ARMTargetParser::getCanonicalArchName(Arch).empty())
       D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+  }
+
+  // We do a similar thing with -mcpu, but here things are complicated because
+  // the only function we have to check if a cpu is valid is
+  // getLLVMArchSuffixForARM which also needs an architecture.
+  if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ)) {
+    StringRef CPU = arm::getARMTargetCPU(Args, Triple);
+    StringRef Arch = arm::getARMArch(Args, Triple);
+    if (strcmp(arm::getLLVMArchSuffixForARM(CPU, Arch), "") == 0)
+      D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+  }
 
   // Setting -msoft-float effectively disables NEON because of the GCC
   // implementation, although the same isn't true of VFP or VFP3.
   if (FloatABI == "soft") {
     Features.push_back("-neon");
     // Also need to explicitly disable features which imply NEON.
     Features.push_back("-crypto");
   }

 
 void Clang::AddARMTargetArgs(const ArgList &Args,
                              ArgStringList &CmdArgs,
                              bool KernelOrKext) const {
   const Driver &D = getToolChain().getDriver();
   // Get the effective triple, which takes into account the deployment target.
   std::string TripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
   llvm::Triple Triple(TripleStr);
-  std::string CPUName = arm::getARMTargetCPU(Args, Triple);
 
   // Select the ABI to use.
   //
   // FIXME: Support -meabi.
   // FIXME: Parts of this are duplicated in the backend, unify this somehow.
   const char *ABIName = nullptr;
   if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ)) {
     ABIName = A->getValue();
   } else if (Triple.isOSBinFormatMachO()) {
     // The backend is hardwired to assume AAPCS for M-class processors, ensure
     // the frontend matches that.
     if (Triple.getEnvironment() == llvm::Triple::EABI ||
         Triple.getOS() == llvm::Triple::UnknownOS ||
-        StringRef(CPUName).startswith("cortex-m")) {
+        isARMMProfile(Triple)) {
       ABIName = "aapcs";
     } else {
       ABIName = "apcs-gnu";
     }
   } else if (Triple.isOSWindows()) {
     // FIXME: this is invalid for WindowsCE
     ABIName = "aapcs";
   } else {
#endif
   (void) Args.hasArg(options::OPT_mtune_EQ);
 
   if (Arg *A = Args.getLastArg(options::OPT_mcmodel_EQ)) {
     CmdArgs.push_back("-mcode-model");
     CmdArgs.push_back(A->getValue());
   }
 
   // Add the target cpu
-  std::string ETripleStr = getToolChain().ComputeEffectiveClangTriple(Args);
-  llvm::Triple ETriple(ETripleStr);
-  std::string CPU = getCPUName(Args, ETriple);
+  std::string CPU = getCPUName(Args, Triple);
   if (!CPU.empty()) {
     CmdArgs.push_back("-target-cpu");
     CmdArgs.push_back(Args.MakeArgString(CPU));
   }
 
   if (const Arg *A = Args.getLastArg(options::OPT_mfpmath_EQ)) {
     CmdArgs.push_back("-mfpmath");
     CmdArgs.push_back(A->getValue());
   }
 
   // Add the target features
-  getTargetFeatures(D, ETriple, Args, CmdArgs, false);
+  getTargetFeatures(D, Triple, Args, CmdArgs, false);
 
   // Add target specific flags.
   switch(getToolChain().getArch()) {
   default:
     break;
 
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
   CmdArgs.push_back("obj");
 
   // Set the main file name, so that debug info works even with
   // -save-temps or preprocessed assembly.
   CmdArgs.push_back("-main-file-name");
   CmdArgs.push_back(Clang::getBaseInputName(Args, Input));
 
   // Add the target cpu
-  const llvm::Triple &Triple = getToolChain().getTriple();
+  const llvm::Triple Triple(TripleStr);
   std::string CPU = getCPUName(Args, Triple);
   if (!CPU.empty()) {
     CmdArgs.push_back("-target-cpu");
     CmdArgs.push_back(Args.MakeArgString(CPU));
   }
 
   // Add the target features
   const Driver &D = getToolChain().getDriver();
const StringRef arm::getARMArch(const ArgList &Args,
   StringRef MArch;
   if (Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
     // Otherwise, if we have -march= choose the base CPU for that arch.
     MArch = A->getValue();
   } else {
     // Otherwise, use the Arch from the triple.
     MArch = Triple.getArchName();
   }
-  return MArch;
-}
-/// Get the (LLVM) name of the minimum ARM CPU for the arch we are targeting.
-const char *arm::getARMCPUForMArch(const ArgList &Args,
-                                   const llvm::Triple &Triple) {
-  StringRef MArch = getARMArch(Args, Triple);
 
   // Handle -march=native.
   if (MArch == "native") {
     std::string CPU = llvm::sys::getHostCPUName();
     if (CPU != "generic") {
-      // Translate the native cpu into the architecture. The switch below will
-      // then chose the minimum cpu for that arch.
-      MArch = std::string("arm") + 
-        arm::getLLVMArchSuffixForARM(CPU, arm::getARMArch(Args, Triple));
+      // Translate the native cpu into the architecture suffix for that CPU.
+      const char *Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+      // If there is no valid architecture suffix for this CPU we don't know how
+      // to handle it, so return no architecture.
+      if (strcmp(Suffix,"") == 0)
+        MArch = "";
+      else
+        MArch = std::string("arm") + Suffix;
     }
   }
 
+  return MArch;
+}
+/// Get the (LLVM) name of the minimum ARM CPU for the arch we are targeting.
+const char *arm::getARMCPUForMArch(const ArgList &Args,
+                                   const llvm::Triple &Triple) {
+  StringRef MArch = getARMArch(Args, Triple);
+  // getARMCPUForArch defaults to the triple if MArch is empty, but empty MArch
+  // here means an -march=native that we can't handle, so instead return no CPU.
+  if (MArch.empty())
+    return "";
+
   // We need to return an empty string here on invalid MArch values as the
   // various places that call this function can't cope with a null result.
   const char *result = Triple.getARMCPUForArch(MArch);
   if (result)
     return result;
   else
     return "";
 }
const char *arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
   // FIXME: Use ARMTargetParser
   if (CPU == "generic") {
     if (Arch == "armv8.1a" || Arch == "armv8.1-a" ||
         Arch == "armebv8.1a" || Arch == "armebv8.1-a") {
       return "v8.1a";
     }
   }
 
+  // FIXME: Use ARMTargetParser
   return llvm::StringSwitch<const char *>(CPU)
-    .Case("strongarm", "v4")
+    .Cases("arm8", "arm810", "v4")
+    .Cases("strongarm", "strongarm110", "strongarm1100", "strongarm1110", "v4")
     .Cases("arm7tdmi", "arm7tdmi-s", "arm710t", "v4t")
     .Cases("arm720t", "arm9", "arm9tdmi", "v4t")
     .Cases("arm920", "arm920t", "arm922t", "v4t")
     .Cases("arm940t", "ep9312","v4t")
     .Cases("arm10tdmi",  "arm1020t", "v5")
     .Cases("arm9e",  "arm926ej-s",  "arm946e-s", "v5e")
     .Cases("arm966e-s",  "arm968e-s",  "arm10e", "v5e")
     .Cases("arm1020e",  "arm1022e",  "xscale", "iwmmxt", "v5e")
return llvm::StringSwitch<const char *>(CPU)
     .Default("");
 }
 
 void arm::appendEBLinkFlags(const ArgList &Args, ArgStringList &CmdArgs, 
                             const llvm::Triple &Triple) {
   if (Args.hasArg(options::OPT_r))
     return;
 
-  StringRef Suffix = getLLVMArchSuffixForARM(getARMCPUForMArch(Args, Triple), 
-                                             getARMArch(Args, Triple));
-  const char *LinkFlag = llvm::StringSwitch<const char *>(Suffix)
-    .Cases("v4", "v4t", "v5", "v5e", nullptr)
-    .Cases("v6", "v6k", "v6t2", nullptr)
-    .Default("--be8");
-
-  if (LinkFlag)
-    CmdArgs.push_back(LinkFlag);
+  // ARMv7 (and later) and ARMv6-M do not support BE-32, so instruct the linker
+  // to generate BE-8 executables.
+  if (getARMSubArchVersionNumber(Triple) >= 7 || isARMMProfile(Triple))
+    CmdArgs.push_back("--be8");
 }
 
 mips::NanEncoding mips::getSupportedNanEncoding(StringRef &CPU) {
   return (NanEncoding)llvm::StringSwitch<int>(CPU)
       .Case("mips1", NanLegacy)
       .Case("mips2", NanLegacy)
       .Case("mips3", NanLegacy)
       .Case("mips4", NanLegacy)
case llvm::Triple::GNUEABIHF:
       break;
     default:
       CmdArgs.push_back("armelf_nbsd");
       break;
     }
     break;
   case llvm::Triple::armeb:
   case llvm::Triple::thumbeb:
-    arm::appendEBLinkFlags(Args, CmdArgs, getToolChain().getTriple());
+    arm::appendEBLinkFlags(Args, CmdArgs,
+        llvm::Triple(getToolChain().ComputeEffectiveClangTriple(Args)));
     CmdArgs.push_back("-m");
     switch (getToolChain().getTriple().getEnvironment()) {
     case llvm::Triple::EABI:
     case llvm::Triple::GNUEABI:
       CmdArgs.push_back("armelfb_nbsd_eabi");
       break;
     case llvm::Triple::EABIHF:
     case llvm::Triple::GNUEABIHF:
void gnutools::Link::ConstructJob(Compilation &C, const JobAction &JA,
   if (Args.hasArg(options::OPT_rdynamic))
     CmdArgs.push_back("-export-dynamic");
 
   if (Args.hasArg(options::OPT_s))
     CmdArgs.push_back("-s");
 
   if (ToolChain.getArch() == llvm::Triple::armeb ||
       ToolChain.getArch() == llvm::Triple::thumbeb)
-    arm::appendEBLinkFlags(Args, CmdArgs, getToolChain().getTriple());
+    arm::appendEBLinkFlags(Args, CmdArgs,
+        llvm::Triple(getToolChain().ComputeEffectiveClangTriple(Args)));
 
   for (const auto &Opt : ToolChain.ExtraOpts)
     CmdArgs.push_back(Opt.c_str());
 
   if (!Args.hasArg(options::OPT_static)) {
     CmdArgs.push_back("--eh-frame-hdr");
   }
 

cfe/trunk/test/Driver/arm-cortex-cpus.c

 // CHECK-BE-V81A-THUMB: "-cc1"{{.*}} "-triple" "thumbebv8.1a-{{.*}}" "-target-cpu" "generic" "-target-feature" "+v8.1a"
 
 // ================== Check that a bogus architecture gives an error
 // RUN: %clang -target arm -march=armbogusv6 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BOGUS %s
 // CHECK-BOGUS: error: the clang compiler does not support '-march=armbogusv6' 
 // RUN: %clang -target arm---eabihf -march=armbogusv7 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BOGUS-HF %s
 // CHECK-BOGUS-HF: error: the clang compiler does not support '-march=armbogusv7' 
 
+// ================== Check that a bogus CPU gives an error
+// RUN: %clang -target arm -mcpu=bogus -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BOGUS-CPU %s
+// CHECK-BOGUS-CPU: error: the clang compiler does not support '-mcpu=bogus' 
+
 // ================== Check default Architecture on each ARM11 CPU
 // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136j-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6 %s
 // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136jf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6 %s
 // CHECK-CPUV6: "-cc1"{{.*}} "-triple" "armv6-{{.*}}
 
 // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s
 // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s
 // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcore -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s