ahatanak updated this revision to Diff 46725.
ahatanak added a comment.

Sorry for taking so long to get back to this patch.

I've updated the patch based on your review comments. I defined a new helper 
function getAArch64FeaturesFromCPU which gets the target features from the cpu 
name and checks whether the cpu is a valid aarch64 cpu. This function is called 
from getAArch64TargetCPU and DecodeAArch64Mcpu to avoid duplicating the code. I 
also added a test case.


http://reviews.llvm.org/D14471

Files:
  lib/Driver/Tools.cpp
  test/Driver/aarch64-cpus.c

Index: test/Driver/aarch64-cpus.c
===================================================================
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -17,6 +17,8 @@
 
 // RUN: %clang -target arm64-apple-darwin -arch arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-DARWIN %s
 // ARM64-DARWIN: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cyclone"
+// RUN: %clang -target arm64-apple-darwin -arch arm64 -mtune=cpu123 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-DARWIN-INVALID-CPU %s
+// ARM64-DARWIN-INVALID-CPU:  error: the clang compiler does not support 'cpu123'
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -1021,31 +1021,65 @@
 }
 // ARM tools end.
 
+/// Get the features from cpu string. Return false to indicate CPU is not a
+/// valid AArch64 cpu.
+static std::pair<std::vector<const char *>, bool>
+getAArch64FeaturesFromCPU(StringRef CPU) {
+  std::vector<const char *> Features;
+  bool IsValid = true;
+
+  if (CPU == "cyclone" || CPU == "cortex-a53" || CPU == "cortex-a57" ||
+      CPU == "cortex-a72" || CPU == "cortex-a35" || CPU == "exynos-m1") {
+    Features.push_back("+neon");
+    Features.push_back("+crc");
+    Features.push_back("+crypto");
+  } else if (CPU == "generic") {
+    Features.push_back("+neon");
+  } else {
+    IsValid = false;
+  }
+
+  return std::make_pair(Features, IsValid);
+}
+
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
-/// targeting.
-static std::string getAArch64TargetCPU(const ArgList &Args) {
+/// targeting. Return false to indicate the cpu is not a valid AArch64 cpu.
+static std::pair<std::string, bool>
+getAArch64TargetCPU(const ArgList &Args) {
   Arg *A;
   std::string CPU;
+  bool IsNative = false;
+
   // If we have -mtune or -mcpu, use that.
   if ((A = Args.getLastArg(options::OPT_mtune_EQ))) {
     CPU = StringRef(A->getValue()).lower();
   } else if ((A = Args.getLastArg(options::OPT_mcpu_EQ))) {
     StringRef Mcpu = A->getValue();
     CPU = Mcpu.split("+").first.lower();
   }
 
-  // Handle CPU name is 'native'.
-  if (CPU == "native")
-    return llvm::sys::getHostCPUName();
-  else if (CPU.size())
-    return CPU;
+  if (CPU.empty()) {
+    // Make sure we pick "cyclone" if -arch is used.
+    // FIXME: Should this be picked by checking the target triple instead?
+    if (Args.getLastArg(options::OPT_arch))
+      return std::make_pair("cyclone", true);
 
-  // Make sure we pick "cyclone" if -arch is used.
-  // FIXME: Should this be picked by checking the target triple instead?
-  if (Args.getLastArg(options::OPT_arch))
-    return "cyclone";
+    return std::make_pair("generic", true);
+  }
 
-  return "generic";
+  // Handle 'native' CPU.
+  if (CPU == "native") {
+    IsNative = true;
+    CPU = llvm::sys::getHostCPUName();
+  }
+
+  // Check if CPU is a valid aarch64 cpu.
+  if (getAArch64FeaturesFromCPU(CPU).second)
+    return std::make_pair(CPU, true);
+
+  // If CPU is not valid, return the cpu name that was passed in -mtune or
+  // -mcpu.
+  return std::make_pair(IsNative ? "native" : CPU, false);
 }
 
 void Clang::AddAArch64TargetArgs(const ArgList &Args,
@@ -1713,7 +1747,7 @@
 
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be:
-    return getAArch64TargetCPU(Args);
+    return getAArch64TargetCPU(Args).first;
 
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
@@ -2128,18 +2162,13 @@
                               std::vector<const char *> &Features) {
   std::pair<StringRef, StringRef> Split = Mcpu.split("+");
   CPU = Split.first;
-  if (CPU == "cyclone" || CPU == "cortex-a53" || CPU == "cortex-a57" ||
-      CPU == "cortex-a72" || CPU == "cortex-a35" || CPU == "exynos-m1") {
-    Features.push_back("+neon");
-    Features.push_back("+crc");
-    Features.push_back("+crypto");
-  } else if (CPU == "generic") {
-    Features.push_back("+neon");
-  } else {
-    return false;
-  }
+  bool IsValidCPU;
 
-  if (Split.second.size() && !DecodeAArch64Features(D, Split.second, Features))
+  std::tie(Features, IsValidCPU) = getAArch64FeaturesFromCPU(CPU);
+
+  if (!IsValidCPU ||
+      (Split.second.size() &&
+       !DecodeAArch64Features(D, Split.second, Features)))
     return false;
 
   return true;
@@ -2214,25 +2243,37 @@
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
+  std::string CPU;
+
   if ((A = Args.getLastArg(options::OPT_march_EQ)))
     success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, Features);
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
     success = getAArch64ArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (Args.hasArg(options::OPT_arch))
-    success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args), Args,
-                                             Features);
+  else if (Args.hasArg(options::OPT_arch)) {
+    std::tie(CPU, success) = getAArch64TargetCPU(Args);
+
+    if (success)
+      success = getAArch64ArchFeaturesFromMcpu(D, CPU, Args, Features);
+    else
+      D.Diag(diag::err_drv_clang_unsupported) << CPU;
+  }
 
   if (success && (A = Args.getLastArg(options::OPT_mtune_EQ)))
     success =
         getAArch64MicroArchFeaturesFromMtune(D, A->getValue(), Args, Features);
   else if (success && (A = Args.getLastArg(options::OPT_mcpu_EQ)))
     success =
         getAArch64MicroArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (Args.hasArg(options::OPT_arch))
-    success = getAArch64MicroArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args),
-                                                  Args, Features);
+  else if (success && Args.hasArg(options::OPT_arch)) {
+    std::tie(CPU, success) = getAArch64TargetCPU(Args);
+
+    if (success)
+      success = getAArch64MicroArchFeaturesFromMcpu(D, CPU, Args, Features);
+    else
+      D.Diag(diag::err_drv_clang_unsupported) << CPU;
+  }
 
-  if (!success)
+  if (!success && A)
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to