<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Sep 23, 2015 at 5:27 PM, Vladimir Sukharev <span dir="ltr"><<a href="mailto:Vladimir.Sukharev@arm.com" target="_blank">Vladimir.Sukharev@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Hi Alexander,
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Sorry, forgot to run valgrind.</span></p></div></div></blockquote><div><br></div><div>valgrind??? I thought, every LLVM developer must know about <a href="http://clang.llvm.org/docs/AddressSanitizer.html">ASAN</a> ;)</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I’m gonna commit quick follow-up (see attach), would you please take a look?</span></p></div></div></blockquote><div><br></div><div>The patch changes the return value of the getLLVMArchSuffixForARM function. Is it intended?<br></div><div><br></div><div>Also, I'd prefer "std::string" instead of "auto".</div><div><br></div><div><br></div><div>-- Alex</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Thanks, Vladimir<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> Alexander Kornienko [mailto:<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>]
<br>
<b>Sent:</b> 23 September 2015 15:41<br>
<b>To:</b> Vladimir Sukharev<br>
<b>Cc:</b> cfe-commits<br>
<b>Subject:</b> Re: r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="<u></u><u></u></span></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">On Wed, Sep 23, 2015 at 11:29 AM, Vladimir Sukharev via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">Author: vsukharev<br>
Date: Wed Sep 23 04:29:32 2015<br>
New Revision: 248370<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=248370&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=248370&view=rev</a><br>
Log:<br>
[ARM] Fix crash "-target arm -mcpu=generic", without "-march="<br>
<br>
An assertion hit has been fixed for cmdlines like<br>
<br>
$ clang --target=arm-linux-gnueabi -mcpu=generic hello.c<br>
<br>
Related to: <a href="http://reviews.llvm.org/rL245445" target="_blank">http://reviews.llvm.org/rL245445</a><br>
<br>
Reviewers: rengolin<br>
<br>
Subscribers: cfe-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D13013" target="_blank">http://reviews.llvm.org/D13013</a><br>
<br>
<br>
Modified:<br>
cfe/trunk/lib/Driver/ToolChain.cpp<br>
cfe/trunk/lib/Driver/Tools.cpp<br>
cfe/trunk/lib/Driver/Tools.h<br>
cfe/trunk/test/Driver/aarch64-cpus.c<br>
cfe/trunk/test/Driver/arm-cortex-cpus.c<br>
<br>
Modified: cfe/trunk/lib/Driver/ToolChain.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=248370&r1=248369&r2=248370&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=248370&r1=248369&r2=248370&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)<br>
+++ cfe/trunk/lib/Driver/ToolChain.cpp Wed Sep 23 04:29:32 2015<br>
@@ -317,8 +317,7 @@ std::string ToolChain::ComputeLLVMTriple<br>
? tools::arm::getARMCPUForMArch(MArch, Triple).str()<br>
: tools::arm::getARMTargetCPU(MCPU, MArch, Triple);<br>
StringRef Suffix =<br>
- tools::arm::getLLVMArchSuffixForARM(CPU,<br>
- tools::arm::getARMArch(MArch, Triple));<br>
+ tools::arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);<br>
bool ThumbDefault = Suffix.startswith("v6m") || Suffix.startswith("v7m") ||<br>
Suffix.startswith("v7em") ||<br>
(Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());<br>
<br>
Modified: cfe/trunk/lib/Driver/Tools.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248370&r1=248369&r2=248370&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=248370&r1=248369&r2=248370&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/Tools.cpp (original)<br>
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep 23 04:29:32 2015<br>
@@ -560,8 +560,7 @@ static void checkARMCPUName(const Driver<br>
llvm::StringRef CPUName, llvm::StringRef ArchName,<br>
const llvm::Triple &Triple) {<br>
std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);<br>
- std::string Arch = arm::getARMArch(ArchName, Triple);<br>
- if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())<br>
+ if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())<br>
D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);<br>
}<br>
<br>
@@ -6087,7 +6086,7 @@ const std::string arm::getARMArch(String<br>
std::string CPU = llvm::sys::getHostCPUName();<br>
if (CPU != "generic") {<br>
// Translate the native cpu into the architecture suffix for that CPU.<br>
- StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);<br>
+ StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);<br>
// If there is no valid architecture suffix for this CPU we don't know how<br>
// to handle it, so return no architecture.<br>
if (Suffix.empty())<br>
@@ -6133,12 +6132,19 @@ std::string arm::getARMTargetCPU(StringR<br>
/// getLLVMArchSuffixForARM - Get the LLVM arch name to use for a particular<br>
/// CPU (or Arch, if CPU is generic).<br>
// FIXME: This is redundant with -mcpu, why does LLVM use this.<br>
-StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {<br>
- if (CPU == "generic")<br>
- return llvm::ARM::getSubArch(<br>
- llvm::ARM::parseArch(Arch));<br>
-<br>
- unsigned ArchKind = llvm::ARM::parseCPUArch(CPU);<br>
+StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,<br>
+ const llvm::Triple &Triple) {<br>
+ unsigned ArchKind;<br>
+ Arch = tools::arm::getARMArch(Arch, Triple);<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">arm::getARMArch returns std::string, so here you're ending up with a dangling StringRef. Please fix or revert.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">+ if (CPU == "generic") {<br>
+ ArchKind = llvm::ARM::parseArch(Arch);<br>
+ if (ArchKind == llvm::ARM::AK_INVALID)<br>
+ // In case of generic Arch, i.e. "arm",<br>
+ // extract arch from default cpu of the Triple<br>
+ ArchKind = llvm::ARM::parseCPUArch(Triple.getARMCPUForArch(Arch));<br>
+ } else {<br>
+ ArchKind = llvm::ARM::parseCPUArch(CPU);<br>
+ }<br>
if (ArchKind == llvm::ARM::AK_INVALID)<br>
return "";<br>
return llvm::ARM::getSubArch(ArchKind);<br>
<br>
Modified: cfe/trunk/lib/Driver/Tools.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.h?rev=248370&r1=248369&r2=248370&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.h?rev=248370&r1=248369&r2=248370&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/Tools.h (original)<br>
+++ cfe/trunk/lib/Driver/Tools.h Wed Sep 23 04:29:32 2015<br>
@@ -246,7 +246,8 @@ std::string getARMTargetCPU(StringRef CP<br>
const std::string getARMArch(StringRef Arch,<br>
const llvm::Triple &Triple);<br>
StringRef getARMCPUForMArch(StringRef Arch, const llvm::Triple &Triple);<br>
-StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch);<br>
+StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,<br>
+ const llvm::Triple &Triple);<br>
<br>
void appendEBLinkFlags(const llvm::opt::ArgList &Args, ArgStringList &CmdArgs,<br>
const llvm::Triple &Triple);<br>
<br>
Modified: cfe/trunk/test/Driver/aarch64-cpus.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-cpus.c?rev=248370&r1=248369&r2=248370&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-cpus.c?rev=248370&r1=248369&r2=248370&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Driver/aarch64-cpus.c (original)<br>
+++ cfe/trunk/test/Driver/aarch64-cpus.c Wed Sep 23 04:29:32 2015<br>
@@ -1,12 +1,17 @@<br>
// Check target CPUs are correctly passed.<br>
<br>
// RUN: %clang -target aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s<br>
+// RUN: %clang -target aarch64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s<br>
// RUN: %clang -target aarch64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s<br>
+// RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s<br>
// RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s<br>
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s<br>
// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"<br>
<br>
// RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s<br>
+// RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s<br>
// RUN: %clang -target arm64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s<br>
+// RUN: %clang -target arm64 -mlittle-endian -mcpu-generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s<br>
<br>
// ARM64-GENERIC: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"<br>
<br>
<br>
Modified: cfe/trunk/test/Driver/arm-cortex-cpus.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=248370&r1=248369&r2=248370&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=248370&r1=248369&r2=248370&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)<br>
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c Wed Sep 23 04:29:32 2015<br>
@@ -1,4 +1,16 @@<br>
// ================== Check default CPU on each major architecture<br>
+// RUN: %clang -target arm -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC %s<br>
+// CHECK-GENERIC: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "generic"<br>
+<br>
+// RUN: %clang -target armeb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC %s<br>
+// CHECK-BE-GENERIC: "-cc1"{{.*}} "-triple" "armebv4t-{{.*}} "-target-cpu" "generic"<br>
+<br>
+// RUN: %clang -target arm -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC-THUMB %s<br>
+// CHECK-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbv4t-{{.*}} "-target-cpu" "generic"<br>
+<br>
+// RUN: %clang -target armeb -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC-THUMB %s<br>
+// CHECK-BE-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbebv4t-{{.*}} "-target-cpu" "generic"<br>
+<br>
// RUN: %clang -target armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s<br>
// RUN: %clang -target arm -march=armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s<br>
// CHECK-V4T: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "arm7tdmi"<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</blockquote>
</div>
<p class="MsoNormal"><br>
<br clear="all">
<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal">-- <u></u><u></u></p>
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="color:rgb(102,102,102);border:1.5pt solid rgb(213,15,37);padding:2pt">Alexander Kornienko |</span><span style="color:rgb(102,102,102);border:1.5pt solid rgb(51,105,232);padding:2pt"> Software Engineer |</span><span style="color:rgb(102,102,102);border:1.5pt solid rgb(0,153,57);padding:2pt"> </span><span style="border:1.5pt solid rgb(0,153,57);padding:2pt"><a href="mailto:alexfh@google.com" target="_blank"><span style="color:rgb(17,85,204)">alexfh@google.com</span></a> |</span><span style="border:1.5pt solid rgb(238,178,17);padding:2pt"> Google
Germany, Munich</span><u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div></div></div>
<br>
<font face="Arial" color="Black" size="2">-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents
to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.<br>
<br>
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590<br>
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782<br>
</font>
</div>
</blockquote></div><br><br clear="all"><div><br></div>
</div></div>