from:"Vladimir Sukharev via cfe\-commits"

[PATCH] D13013: Fix crash "-target arm -mcpu=generic", without "-march="

2015-09-21 Thread Vladimir Sukharev via cfe-commits

vsukharev created this revision.
vsukharev added a reviewer: rengolin.
vsukharev added a subscriber: cfe-commits.
vsukharev set the repository for this revision to rL LLVM.
Herald added subscribers: rengolin, aemerson.

Follow-up to http://reviews.llvm.org/rL245445
Added more tests for "-mcpu=generic", a crash fixed.


Repository:
  rL LLVM

http://reviews.llvm.org/D13013

Files:
  lib/Driver/ToolChain.cpp
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  test/Driver/aarch64-cpus.c
  test/Driver/arm-cortex-cpus.c

Index: test/Driver/arm-cortex-cpus.c
===
--- test/Driver/arm-cortex-cpus.c
+++ test/Driver/arm-cortex-cpus.c
@@ -1,4 +1,16 @@
 // == Check default CPU on each major architecture
+// RUN: %clang -target arm -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC %s
+// CHECK-GENERIC: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target armeb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC %s
+// CHECK-BE-GENERIC: "-cc1"{{.*}} "-triple" "armebv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target arm -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC-THUMB %s
+// CHECK-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target armeb -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC-THUMB %s
+// CHECK-BE-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbebv4t-{{.*}} "-target-cpu" "generic"
+
 // RUN: %clang -target armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s
 // RUN: %clang -target arm -march=armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s
 // CHECK-V4T: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "arm7tdmi"
Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -1,12 +1,17 @@
 // Check target CPUs are correctly passed.
 
 // RUN: %clang -target aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
+// RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
+// RUN: %clang -target arm64 -mlittle-endian -mcpu-generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 
 // ARM64-GENERIC: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
 
Index: lib/Driver/Tools.h
===
--- lib/Driver/Tools.h
+++ lib/Driver/Tools.h
@@ -246,7 +246,8 @@
 const std::string getARMArch(StringRef Arch,
  const llvm::Triple &Triple);
 StringRef getARMCPUForMArch(StringRef Arch, const llvm::Triple &Triple);
-StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch);
+StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+  const llvm::Triple &Triple);
 
 void appendEBLinkFlags(const llvm::opt::ArgList &Args, ArgStringList &CmdArgs,
const llvm::Triple &Triple);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -560,8 +560,7 @@
 llvm::StringRef CPUName, llvm::StringRef ArchName,
 const llvm::Triple &Triple) {
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-  std::string Arch = arm::getARMArch(ArchName, Triple);
-  if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())
+  if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
@@ -6081,7 +6080,7 @@
 std::string CPU = llvm::sys::getHostCPUName();
 if (CPU != "generic") {
   // Translate the native cpu into the architecture suffix for that CPU.
-  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
   // If there is no valid architecture suffix for this CPU we don't know how
   // to handle it, so

Re: [PATCH] D13013: [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

2015-09-21 Thread Vladimir Sukharev via cfe-commits

vsukharev added a comment.

Yes, trying to pass new tests in test/Driver/arm-cortex-cpus.c, without code 
patch, it crashes

  $ clang --target=arm-linux-gnueabi -mcpu=generic hello.c
  /work/oss_llvm/llvm/include/llvm/ADT/StringRef.h:84: 
llvm::StringRef::StringRef(const char*, size_t): Assertion `(data || length == 
0) && "StringRef cannot be built from a NULL argument with non-null length"' 
failed.
  ...
  #9 0x2b22f25 (anonymous namespace)::._115::getSubArch() const 
lib/Support/TargetParser.cpp:74:0
  #10 0x2b27147 llvm::ARM::getSubArch(unsigned int) 
lib/Support/TargetParser.cpp:309:0
  #11 0x31871a1 
clang::driver::tools::arm::getLLVMArchSuffixForARM(llvm::StringRef, 
llvm::StringRef) tools/clang/lib/Driver/Tools.cpp:6374:0
  #12 0x3125da8 clang::driver::ToolChain::ComputeLLVMTriple(llvm::opt::ArgList 
const&, clang::driver::types::ID) const 
tools/clang/lib/Driver/ToolChain.cpp:321:0
  #13 0x312656c 
clang::driver::ToolChain::ComputeEffectiveClangTriple(llvm::opt::ArgList 
const&, clang::driver::types::ID) const 
tools/clang/lib/Driver/ToolChain.cpp:353:0
  #14 0x31726b1 
clang::driver::tools::Clang::ConstructJob(clang::driver::Compilation&, 
clang::driver::JobAction const&, clang::driver::InputInfo const&, 
llvm::SmallVector const&, llvm::opt::ArgList 
const&, char const*) const tools/clang/lib/Driver/Tools.cpp:3363:0


Repository:
  rL LLVM

http://reviews.llvm.org/D13013



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

2015-09-23 Thread Vladimir Sukharev via cfe-commits

Author: vsukharev
Date: Wed Sep 23 04:29:32 2015
New Revision: 248370

URL: http://llvm.org/viewvc/llvm-project?rev=248370&view=rev
Log:
[ARM] Fix crash "-target arm -mcpu=generic", without "-march="

An assertion hit has been fixed for cmdlines like

$ clang --target=arm-linux-gnueabi -mcpu=generic hello.c

Related to: http://reviews.llvm.org/rL245445

Reviewers: rengolin

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D13013


Modified:
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Driver/Tools.h
cfe/trunk/test/Driver/aarch64-cpus.c
cfe/trunk/test/Driver/arm-cortex-cpus.c

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=248370&r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Wed Sep 23 04:29:32 2015
@@ -317,8 +317,7 @@ std::string ToolChain::ComputeLLVMTriple
 ? tools::arm::getARMCPUForMArch(MArch, Triple).str()
 : tools::arm::getARMTargetCPU(MCPU, MArch, Triple);
 StringRef Suffix =
-  tools::arm::getLLVMArchSuffixForARM(CPU,
-  tools::arm::getARMArch(MArch, 
Triple));
+  tools::arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
 bool ThumbDefault = Suffix.startswith("v6m") || Suffix.startswith("v7m") ||
   Suffix.startswith("v7em") ||
   (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248370&r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep 23 04:29:32 2015
@@ -560,8 +560,7 @@ static void checkARMCPUName(const Driver
 llvm::StringRef CPUName, llvm::StringRef ArchName,
 const llvm::Triple &Triple) {
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-  std::string Arch = arm::getARMArch(ArchName, Triple);
-  if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())
+  if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
@@ -6087,7 +6086,7 @@ const std::string arm::getARMArch(String
 std::string CPU = llvm::sys::getHostCPUName();
 if (CPU != "generic") {
   // Translate the native cpu into the architecture suffix for that CPU.
-  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
   // If there is no valid architecture suffix for this CPU we don't know 
how
   // to handle it, so return no architecture.
   if (Suffix.empty())
@@ -6133,12 +6132,19 @@ std::string arm::getARMTargetCPU(StringR
 /// getLLVMArchSuffixForARM - Get the LLVM arch name to use for a particular
 /// CPU  (or Arch, if CPU is generic).
 // FIXME: This is redundant with -mcpu, why does LLVM use this.
-StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
-  if (CPU == "generic")
-return llvm::ARM::getSubArch(
-llvm::ARM::parseArch(Arch));
-
-  unsigned ArchKind = llvm::ARM::parseCPUArch(CPU);
+StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+   const llvm::Triple &Triple) {
+  unsigned ArchKind;
+  Arch = tools::arm::getARMArch(Arch, Triple);
+  if (CPU == "generic") {
+ArchKind = llvm::ARM::parseArch(Arch);
+if (ArchKind == llvm::ARM::AK_INVALID)
+  // In case of generic Arch, i.e. "arm",
+  // extract arch from default cpu of the Triple
+  ArchKind = llvm::ARM::parseCPUArch(Triple.getARMCPUForArch(Arch));
+  } else {
+ArchKind = llvm::ARM::parseCPUArch(CPU);
+  }
   if (ArchKind == llvm::ARM::AK_INVALID)
 return "";
   return llvm::ARM::getSubArch(ArchKind);

Modified: cfe/trunk/lib/Driver/Tools.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.h?rev=248370&r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/Tools.h (original)
+++ cfe/trunk/lib/Driver/Tools.h Wed Sep 23 04:29:32 2015
@@ -246,7 +246,8 @@ std::string getARMTargetCPU(StringRef CP
 const std::string getARMArch(StringRef Arch,
  const llvm::Triple &Triple);
 StringRef getARMCPUForMArch(StringRef Arch, const llvm::Triple &Triple);
-StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch);
+StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+  const llvm::Triple &Triple);
 
 void appendEBLinkFlags(const llvm::opt::ArgList &Args, ArgStringList &CmdArg

Re: [PATCH] D13013: [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

2015-09-23 Thread Vladimir Sukharev via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL248370: [ARM] Fix crash "-target arm -mcpu=generic", without 
"-march=" (authored by vsukharev).

Changed prior to commit:
  http://reviews.llvm.org/D13013?vs=35234&id=35472#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D13013

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/lib/Driver/Tools.h
  cfe/trunk/test/Driver/aarch64-cpus.c
  cfe/trunk/test/Driver/arm-cortex-cpus.c

Index: cfe/trunk/test/Driver/arm-cortex-cpus.c
===
--- cfe/trunk/test/Driver/arm-cortex-cpus.c
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c
@@ -1,4 +1,16 @@
 // == Check default CPU on each major architecture
+// RUN: %clang -target arm -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC %s
+// CHECK-GENERIC: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target armeb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC %s
+// CHECK-BE-GENERIC: "-cc1"{{.*}} "-triple" "armebv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target arm -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC-THUMB %s
+// CHECK-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target armeb -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC-THUMB %s
+// CHECK-BE-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbebv4t-{{.*}} "-target-cpu" "generic"
+
 // RUN: %clang -target armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s
 // RUN: %clang -target arm -march=armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s
 // CHECK-V4T: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "arm7tdmi"
Index: cfe/trunk/test/Driver/aarch64-cpus.c
===
--- cfe/trunk/test/Driver/aarch64-cpus.c
+++ cfe/trunk/test/Driver/aarch64-cpus.c
@@ -1,12 +1,17 @@
 // Check target CPUs are correctly passed.
 
 // RUN: %clang -target aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
+// RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
+// RUN: %clang -target arm64 -mlittle-endian -mcpu-generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 
 // ARM64-GENERIC: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
 
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -560,8 +560,7 @@
 llvm::StringRef CPUName, llvm::StringRef ArchName,
 const llvm::Triple &Triple) {
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-  std::string Arch = arm::getARMArch(ArchName, Triple);
-  if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())
+  if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
@@ -6087,7 +6086,7 @@
 std::string CPU = llvm::sys::getHostCPUName();
 if (CPU != "generic") {
   // Translate the native cpu into the architecture suffix for that CPU.
-  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
   // If there is no valid architecture suffix for this CPU we don't know how
   // to handle it, so return no architecture.
   if (Suffix.empty())
@@ -6133,12 +6132,19 @@
 /// getLLVMArchSuffixForARM - Get the LLVM arch name to use for a particular
 /// CPU  (or Arch, if CPU is generic).
 // FIXME: This is redundant with -mcpu, why does LLVM use this.
-StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
-  if (CPU == "generic")
-return llvm::ARM::getSubArch(
-llvm::ARM::parseArch(Arch));
-
-  unsigned ArchKind = llvm::ARM::parseCPUArch(CPU);
+StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+

RE: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

2015-09-23 Thread Vladimir Sukharev via cfe-commits

Hi Alexander,
Sorry, forgot to run valgrind.
I’m gonna commit quick follow-up (see attach), would you please take a look?

Thanks, Vladimir


From: Alexander Kornienko [mailto:ale...@google.com]
Sent: 23 September 2015 15:41
To: Vladimir Sukharev
Cc: cfe-commits
Subject: Re: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without 
"-march="

On Wed, Sep 23, 2015 at 11:29 AM, Vladimir Sukharev via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: vsukharev
Date: Wed Sep 23 04:29:32 2015
New Revision: 248370

URL: http://llvm.org/viewvc/llvm-project?rev=248370&view=rev
Log:
[ARM] Fix crash "-target arm -mcpu=generic", without "-march="

An assertion hit has been fixed for cmdlines like

$ clang --target=arm-linux-gnueabi -mcpu=generic hello.c

Related to: http://reviews.llvm.org/rL245445

Reviewers: rengolin

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D13013


Modified:
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Driver/Tools.h
cfe/trunk/test/Driver/aarch64-cpus.c
cfe/trunk/test/Driver/arm-cortex-cpus.c

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=248370&r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Wed Sep 23 04:29:32 2015
@@ -317,8 +317,7 @@ std::string ToolChain::ComputeLLVMTriple
 ? tools::arm::getARMCPUForMArch(MArch, Triple).str()
 : tools::arm::getARMTargetCPU(MCPU, MArch, Triple);
 StringRef Suffix =
-  tools::arm::getLLVMArchSuffixForARM(CPU,
-  tools::arm::getARMArch(MArch, 
Triple));
+  tools::arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
 bool ThumbDefault = Suffix.startswith("v6m") || Suffix.startswith("v7m") ||
   Suffix.startswith("v7em") ||
   (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248370&r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep 23 04:29:32 2015
@@ -560,8 +560,7 @@ static void checkARMCPUName(const Driver
 llvm::StringRef CPUName, llvm::StringRef ArchName,
 const llvm::Triple &Triple) {
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-  std::string Arch = arm::getARMArch(ArchName, Triple);
-  if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())
+  if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }

@@ -6087,7 +6086,7 @@ const std::string arm::getARMArch(String
 std::string CPU = llvm::sys::getHostCPUName();
 if (CPU != "generic") {
   // Translate the native cpu into the architecture suffix for that CPU.
-  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
   // If there is no valid architecture suffix for this CPU we don't know 
how
   // to handle it, so return no architecture.
   if (Suffix.empty())
@@ -6133,12 +6132,19 @@ std::string arm::getARMTargetCPU(StringR
 /// getLLVMArchSuffixForARM - Get the LLVM arch name to use for a particular
 /// CPU  (or Arch, if CPU is generic).
 // FIXME: This is redundant with -mcpu, why does LLVM use this.
-StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
-  if (CPU == "generic")
-return llvm::ARM::getSubArch(
-llvm::ARM::parseArch(Arch));
-
-  unsigned ArchKind = llvm::ARM::parseCPUArch(CPU);
+StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+   const llvm::Triple &Triple) {
+  unsigned ArchKind;
+  Arch = tools::arm::getARMArch(Arch, Triple);

arm::getARMArch returns std::string, so here you're ending up with a dangling 
StringRef. Please fix or revert.

+  if (CPU == "generic") {
+ArchKind = llvm::ARM::parseArch(Arch);
+if (ArchKind == llvm::ARM::AK_INVALID)
+  // In case of generic Arch, i.e. "arm",
+  // extract arch from default cpu of the Triple
+  ArchKind = llvm::ARM::parseCPUArch(Triple.getARMCPUForArch(Arch));
+  } else {
+ArchKind = llvm::ARM::parseCPUArch(CPU);
+  }
   if (ArchKind == llvm::ARM::AK_INVALID)
 return "";
   return llvm::ARM::getSubArch(ArchKind);

Modified: cfe/trunk/lib/Driver/Tools.h
URL: 
http://llvm.org/vi

RE: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

2015-09-23 Thread Vladimir Sukharev via cfe-commits

 

Ø  valgrind??? I thought, every LLVM developer must know about ASAN 
<http://clang.llvm.org/docs/AddressSanitizer.html>  ;)

ASAN is not that great to quicktest such a small patches, and valgrind does its 
job in plug-and play manner.

Or, which is recommended way to use ASAN as a pre-commit test, apart from 
obvious “always build clang with ASAN” ? Any docs, tips/tricks?

 

 

Ø  The patch changes the return value of the getLLVMArchSuffixForARM function. 
Is it intended?

Not sure what do you mean. It seems I haven’t changed the semantics during this 
fix. If you’re asking about http://reviews.llvm.org/rL248370 , would you please 
post comment there?

 

Ø  Also, I'd prefer "std::string" instead of "auto".

Ok, next version is attached

 

 

-- Alex

 

 

Thanks, Vladimir

 

 

From: Alexander Kornienko [mailto:ale...@google.com] 
Sent: 23 September 2015 15:41
To: Vladimir Sukharev
Cc: cfe-commits
Subject: Re: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without 
"-march="

 

On Wed, Sep 23, 2015 at 11:29 AM, Vladimir Sukharev via cfe-commits 
 wrote:

Author: vsukharev
Date: Wed Sep 23 04:29:32 2015
New Revision: 248370

URL: http://llvm.org/viewvc/llvm-project?rev=248370 
<http://llvm.org/viewvc/llvm-project?rev=248370&view=rev> &view=rev
Log:
[ARM] Fix crash "-target arm -mcpu=generic", without "-march="

An assertion hit has been fixed for cmdlines like

$ clang --target=arm-linux-gnueabi -mcpu=generic hello.c

Related to: http://reviews.llvm.org/rL245445

Reviewers: rengolin

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D13013


Modified:
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Driver/Tools.h
cfe/trunk/test/Driver/aarch64-cpus.c
cfe/trunk/test/Driver/arm-cortex-cpus.c

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=248370
 
<http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=248370&r1=248369&r2=248370&view=diff>
 &r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Wed Sep 23 04:29:32 2015
@@ -317,8 +317,7 @@ std::string ToolChain::ComputeLLVMTriple
 ? tools::arm::getARMCPUForMArch(MArch, Triple).str()
 : tools::arm::getARMTargetCPU(MCPU, MArch, Triple);
 StringRef Suffix =
-  tools::arm::getLLVMArchSuffixForARM(CPU,
-  tools::arm::getARMArch(MArch, 
Triple));
+  tools::arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
 bool ThumbDefault = Suffix.startswith("v6m") || Suffix.startswith("v7m") ||
   Suffix.startswith("v7em") ||
   (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248370 
<http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248370&r1=248369&r2=248370&view=diff>
 &r1=248369&r2=248370&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep 23 04:29:32 2015
@@ -560,8 +560,7 @@ static void checkARMCPUName(const Driver
 llvm::StringRef CPUName, llvm::StringRef ArchName,
 const llvm::Triple &Triple) {
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-  std::string Arch = arm::getARMArch(ArchName, Triple);
-  if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())
+  if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }

@@ -6087,7 +6086,7 @@ const std::string arm::getARMArch(String
 std::string CPU = llvm::sys::getHostCPUName();
 if (CPU != "generic") {
   // Translate the native cpu into the architecture suffix for that CPU.
-  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+  StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
   // If there is no valid architecture suffix for this CPU we don't know 
how
   // to handle it, so return no architecture.
   if (Suffix.empty())
@@ -6133,12 +6132,19 @@ std::string arm::getARMTargetCPU(StringR
 /// getLLVMArchSuffixForARM - Get the LLVM arch name to use for a particular
 /// CPU  (or Arch, if CPU is generic).
 // FIXME: This is redundant with -mcpu, why does LLVM use this.
-StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
-  if (CPU == "generic")
-return llvm::ARM::get

r248479 - [ARM] Follow-up to fix crash "-target arm -mcpu=generic", without "-march="

2015-09-24 Thread Vladimir Sukharev via cfe-commits

Author: vsukharev
Date: Thu Sep 24 04:55:08 2015
New Revision: 248479

URL: http://llvm.org/viewvc/llvm-project?rev=248479&view=rev
Log:
[ARM] Follow-up to fix crash "-target arm -mcpu=generic", without "-march="

Fix of dangling StringRef after temporary std::string is destroyed

Follow-up to: http://reviews.llvm.org/rL248370

Reviewers: alexfh

Subscribers: cfe-commits


Modified:
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248479&r1=248478&r2=248479&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Sep 24 04:55:08 2015
@@ -6164,13 +6164,13 @@ std::string arm::getARMTargetCPU(StringR
 StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
const llvm::Triple &Triple) {
   unsigned ArchKind;
-  Arch = tools::arm::getARMArch(Arch, Triple);
   if (CPU == "generic") {
-ArchKind = llvm::ARM::parseArch(Arch);
+StringRef ARMArch = tools::arm::getARMArch(Arch, Triple);
+ArchKind = llvm::ARM::parseArch(ARMArch);
 if (ArchKind == llvm::ARM::AK_INVALID)
   // In case of generic Arch, i.e. "arm",
   // extract arch from default cpu of the Triple
-  ArchKind = llvm::ARM::parseCPUArch(Triple.getARMCPUForArch(Arch));
+  ArchKind = llvm::ARM::parseCPUArch(Triple.getARMCPUForArch(ARMArch));
   } else {
 ArchKind = llvm::ARM::parseCPUArch(CPU);
   }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r248480 - [ARM] Follow-up to fix crash "-target arm -mcpu=generic", without "-march="

2015-09-24 Thread Vladimir Sukharev via cfe-commits

Author: vsukharev
Date: Thu Sep 24 05:06:44 2015
New Revision: 248480

URL: http://llvm.org/viewvc/llvm-project?rev=248480&view=rev
Log:
[ARM] Follow-up to fix crash "-target arm -mcpu=generic", without "-march="

Fix of dangling StringRef after temporary std::string is destroyed

Follow-up to: http://reviews.llvm.org/rL248479

Reviewers: alexfh

Subscribers: cfe-commits


Modified:
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248480&r1=248479&r2=248480&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Sep 24 05:06:44 2015
@@ -6165,7 +6165,7 @@ StringRef arm::getLLVMArchSuffixForARM(S
const llvm::Triple &Triple) {
   unsigned ArchKind;
   if (CPU == "generic") {
-StringRef ARMArch = tools::arm::getARMArch(Arch, Triple);
+std::string ARMArch = tools::arm::getARMArch(Arch, Triple);
 ArchKind = llvm::ARM::parseArch(ARMArch);
 if (ArchKind == llvm::ARM::AK_INVALID)
   // In case of generic Arch, i.e. "arm",


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r245445 - [ARM] Proper generic cpus handling

2015-08-19 Thread Vladimir Sukharev via cfe-commits

Author: vsukharev
Date: Wed Aug 19 09:50:18 2015
New Revision: 245445

URL: http://llvm.org/viewvc/llvm-project?rev=245445&view=rev
Log:
[ARM] Proper generic cpus handling

"generic" cpu was wrongly handled as exact real CPU name of ARMv8.1A 
architecture.

This has been fixed, now it is abstract name, suitable for any arch.

Reviewers: rengolin

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D11640

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/arm-cortex-cpus.c

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=245445&r1=245444&r2=245445&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Wed Aug 19 09:50:18 2015
@@ -4257,21 +4257,18 @@ class ARMTargetInfo : public TargetInfo
 ArchISA= llvm::ARMTargetParser::parseArchISA(ArchName);
 DefaultCPU = getDefaultCPU(ArchName);
 
-// SubArch is specified by the target triple
-if (!DefaultCPU.empty()) 
-  setArchInfo(DefaultCPU);
-else 
-  // FIXME ArchInfo should be based on ArchName from triple, not on 
-  // a hard-coded CPU name. Doing so currently causes regressions:
-  // test/Preprocessor/init.c: __ARM_ARCH_6J__ not defined
-  setArchInfo(CPU);
+unsigned ArchKind = llvm::ARMTargetParser::parseArch(ArchName);
+if (ArchKind == llvm::ARM::AK_INVALID)
+  // set arch of the CPU, either provided explicitly or hardcoded default
+  ArchKind = llvm::ARMTargetParser::parseCPUArch(CPU);
+setArchInfo(ArchKind);
   }
 
-  void setArchInfo(StringRef CPU) {
+  void setArchInfo(unsigned Kind) {
 StringRef SubArch;
 
 // cache TargetParser info
-ArchKind= llvm::ARMTargetParser::parseCPUArch(CPU);
+ArchKind= Kind;
 SubArch = llvm::ARMTargetParser::getSubArch(ArchKind);
 ArchProfile = llvm::ARMTargetParser::parseArchProfile(SubArch);
 ArchVersion = llvm::ARMTargetParser::parseArchVersion(SubArch);
@@ -4570,10 +4567,11 @@ public:
   }
 
   bool setCPU(const std::string &Name) override {
-unsigned ArchKind = llvm::ARMTargetParser::parseCPUArch(Name);
+if (Name != "generic")
+  setArchInfo(llvm::ARMTargetParser::parseCPUArch(Name));
+
 if (ArchKind == llvm::ARM::AK_INVALID)
   return false;
-setArchInfo(Name);
 setAtomic();
 CPU = Name;
 return true;

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=245445&r1=245444&r2=245445&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Aug 19 09:50:18 2015
@@ -6002,9 +6002,9 @@ std::string arm::getARMTargetCPU(StringR
 /// CPU  (or Arch, if CPU is generic).
 // FIXME: This is redundant with -mcpu, why does LLVM use this.
 const char *arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
-  if (CPU == "generic" &&
-  llvm::ARMTargetParser::parseArch(Arch) == llvm::ARM::AK_ARMV8_1A)
-return "v8.1a";
+  if (CPU == "generic")
+return llvm::ARMTargetParser::getSubArch(
+llvm::ARMTargetParser::parseArch(Arch));
 
   unsigned ArchKind = llvm::ARMTargetParser::parseCPUArch(CPU);
   if (ArchKind == llvm::ARM::AK_INVALID)

Modified: cfe/trunk/test/Driver/arm-cortex-cpus.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=245445&r1=245444&r2=245445&view=diff
==
--- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c Wed Aug 19 09:50:18 2015
@@ -134,6 +134,18 @@
 // RUN: %clang -target arm -mlittle-endian -march=armv8-a -mlittle-endian -### 
-c %s 2>&1 | FileCheck -check-prefix=CHECK-V8A %s
 // CHECK-V8A: "-cc1"{{.*}} "-triple" "armv8-{{.*}}" "-target-cpu" "cortex-a53"
 
+// RUN: %clang -mcpu=generic -target armv8 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target arm -march=armv8 -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target armv8a -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target arm -march=armv8a -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target arm -march=armv8-a -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target armv8 -mlittle-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target arm -march=armv8 -mlittle-endian -### -c 
%s 2>&1 | FileCheck -check-prefix=CHECK-V8A-GENERIC %s
+// RUN: %clang -mcpu=generic -target armv8a -mlittle-endian -### -c %s 2>&1 | 
Fil

[PATCH] D13013: Fix crash "-target arm -mcpu=generic", without "-march="

Re: [PATCH] D13013: [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

Re: [PATCH] D13013: [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

RE: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

RE: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="

r248479 - [ARM] Follow-up to fix crash "-target arm -mcpu=generic", without "-march="

r248480 - [ARM] Follow-up to fix crash "-target arm -mcpu=generic", without "-march="

r245445 - [ARM] Proper generic cpus handling

9 matches

Site Navigation

Mail list logo

Footer information