Allow case-insensitive values for -mcpu for ARM in line with GCC.

[enefaim]

Summary

GCC allows case-insensitive values for -mcpu, -march and -mtune options.
This patch implements the same behaviour for the -mcpu option.

This is a new version of r236859, which has been reverted due to test failures
in the AArch64 code path when running on 32 ARM buildbot.
I separated the ARM code path into this patch, while debugging the AArch64 failures.

[richard.barton.arm]

My only observation is that you have changed the Krait codepath, but have not added a test for that.

Otherwise LGTM.

[craig.topper]

+ StringRef MCPU = StringRef(A->getValue()).lower();

lower returns an std::string which will be immediately deleted leaving the
MCPU StringRef pointing at stale memory. MCPU should be a std::string.

[rengolin]

I agree a test for krait is needed.

cheers,
--renato

lib/Driver/Tools.cpp

5755

Voicing Craig Topper's comment on the list:

"lower returns an std::string which will be immediately deleted leaving the MCPU StringRef pointing at stale memory. MCPU should be a std::string"

[enefaim]

Changes in this version:

• Added test cases for krait
• Switched getARMTargetCPU to use std::string instead of StringRef

[rengolin]

lib/Driver/Tools.cpp

5750

Don't you need to change the type in the callers, too? Otherwise, the StringRef will refer to a temp variable that may disappear until it's used.

test/Driver/krait-cpu.c

11

You don't need a new CHECK line type, you could have re-used CHECK-CPUV7A

[enefaim]

Changes in this version:

• Change StringRef to std::string in getARMTargetCPU callers to avoid possible memory corruptions.
• Remove unnecessary CHECK line from test/Driver/krait-cpu.c.

[rengolin]

LGTM, Thanks!

[rengolin]

r239059

lib/Driver/ToolChain.cpp

     if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
                                  options::OPT_mbig_endian)) {
       IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
     }
 
     // Thumb2 is the default for V7 on Darwin.
     //
     // FIXME: Thumb should just be another -target-feaure, not in the triple.
-    StringRef CPU = Triple.isOSBinFormatMachO()
+    std::string CPU = Triple.isOSBinFormatMachO()
       ? tools::arm::getARMCPUForMArch(Args, Triple)
       : tools::arm::getARMTargetCPU(Args, Triple);
     StringRef Suffix = 
       tools::arm::getLLVMArchSuffixForARM(CPU,
                                           tools::arm::getARMArch(Args, Triple));
     bool ThumbDefault = Suffix.startswith("v6m") || Suffix.startswith("v7m") ||
       Suffix.startswith("v7em") ||
       (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());

lib/Driver/Tools.h

     void ConstructJob(Compilation &C, const JobAction &JA,
                       const InputInfo &Output, const InputInfoList &Inputs,
                       const llvm::opt::ArgList &TCArgs,
                       const char *LinkingOutput) const override;
   };
 } // end namespace hexagon.
 
 namespace arm {
-  StringRef getARMTargetCPU(const llvm::opt::ArgList &Args,
+  std::string getARMTargetCPU(const llvm::opt::ArgList &Args,
                               const llvm::Triple &Triple);
   const StringRef getARMArch(const llvm::opt::ArgList &Args,
                              const llvm::Triple &Triple);
   const char* getARMCPUForMArch(const llvm::opt::ArgList &Args,
                                 const llvm::Triple &Triple);
   const char* getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch);
 
   void appendEBLinkFlags(const llvm::opt::ArgList &Args, ArgStringList &CmdArgs, const llvm::Triple &Triple);
 }

lib/Driver/Tools.cpp

     if (llvm::ARMTargetParser::parseArch(Arch) == llvm::ARM::AK_INVALID)
       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);
+    std::string 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") {

   const char *result = Triple.getARMCPUForArch(MArch);
   if (result)
     return result;
   else
     return "";
 }
 
 /// getARMTargetCPU - Get the (LLVM) name of the ARM cpu we are targeting.
-StringRef arm::getARMTargetCPU(const ArgList &Args,
+std::string arm::getARMTargetCPU(const ArgList &Args,
                                const llvm::Triple &Triple) {
   // FIXME: Warn on inconsistent use of -mcpu and -march.
   // If we have -mcpu=, use that.
   if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ)) {
-    StringRef MCPU = A->getValue();
+    std::string MCPU = StringRef(A->getValue()).lower();
     // Handle -mcpu=native.
     if (MCPU == "native")
       return llvm::sys::getHostCPUName();
     else
       return MCPU;
   }
 
   return getARMCPUForMArch(Args, Triple);

   switch (getToolChain().getArch()) {
   case llvm::Triple::x86:
     CmdArgs.push_back("--32");
     break;
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb: {
-    std::string MArch(arm::getARMTargetCPU(Args, getToolChain().getTriple()));
+    std::string MArch = arm::getARMTargetCPU(Args, getToolChain().getTriple());
     CmdArgs.push_back(Args.MakeArgString("-mcpu=" + MArch));
     break;
   }
 
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el: {

 
     Args.AddLastArg(CmdArgs, options::OPT_march_EQ);
 
     // FIXME: remove krait check when GNU tools support krait cpu
     // for now replace it with -march=armv7-a  to avoid a lower
     // march from being picked in the absence of a cpu flag.
     Arg *A;
     if ((A = Args.getLastArg(options::OPT_mcpu_EQ)) &&
-      StringRef(A->getValue()) == "krait")
+      StringRef(A->getValue()).lower() == "krait")
         CmdArgs.push_back("-march=armv7-a");
     else
       Args.AddLastArg(CmdArgs, options::OPT_mcpu_EQ);
     Args.AddLastArg(CmdArgs, options::OPT_mfpu_EQ);
     break;
   }
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:

test/Driver/arm-cortex-cpus.c

 
 // RUN: %clang -target armeb -mcpu=cortex-a53 -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV8A-THUMB %s
 // RUN: %clang -target armeb -mcpu=cortex-a57 -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV8A-THUMB %s
 // RUN: %clang -target armeb -mcpu=cortex-a72 -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV8A-THUMB %s
 // RUN: %clang -target arm -mcpu=cortex-a53 -mbig-endian -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV8A-THUMB %s
 // RUN: %clang -target arm -mcpu=cortex-a57 -mbig-endian -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV8A-THUMB %s
 // RUN: %clang -target arm -mcpu=cortex-a72 -mbig-endian -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV8A-THUMB %s
 // CHECK-BE-CPUV8A-THUMB: "-cc1"{{.*}} "-triple" "thumbebv8-{{.*}}
+
+// ================== Check whether -mcpu accepts mixed-case values.
+// RUN: %clang -target arm-linux-gnueabi -mcpu=Cortex-a5 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=cortex-A7 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=CORTEX-a8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=Cortex-A9 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=corteX-A12 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=CorteX-a15 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=CorteX-A17 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
+// CHECK-CASE-INSENSITIVE-CPUV7A: "-cc1"{{.*}} "-triple" "armv7-{{.*}}

test/Driver/krait-cpu.c

 // ================== Check default Architecture on krait CPU
 // RUN: %clang -target arm-linux-gnueabi -mcpu=krait -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV7A %s
+// ================== Check whether -mcpu accepts mixed-case values.
+// RUN: %clang -target arm-linux-gnueabi -mcpu=Krait -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV7A %s
+// RUN: %clang -target arm-linux-gnueabi -mcpu=KRAIT -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV7A %s
 // CHECK-CPUV7A: "-cc1"{{.*}} "-triple" "armv7-{{.*}}