r248370 - [ARM] Fix crash "-target arm -mcpu=generic", without "-march="
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 23 08:49:56 PDT 2015
On Wed, Sep 23, 2015 at 5:27 PM, Vladimir Sukharev <
Vladimir.Sukharev at arm.com> wrote:
> Hi Alexander,
>
> Sorry, forgot to run valgrind.
>
valgrind??? I thought, every LLVM developer must know about ASAN
<http://clang.llvm.org/docs/AddressSanitizer.html> ;)
> I’m gonna commit quick follow-up (see attach), would you please take a
> look?
>
The patch changes the return value of the getLLVMArchSuffixForARM function.
Is it intended?
Also, I'd prefer "std::string" instead of "auto".
-- Alex
>
>
> Thanks, Vladimir
>
>
>
>
>
> *From:* Alexander Kornienko [mailto:alexfh at 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 <
> cfe-commits at 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/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
> &CmdArgs,
> const llvm::Triple &Triple);
>
> Modified: cfe/trunk/test/Driver/aarch64-cpus.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-cpus.c?rev=248370&r1=248369&r2=248370&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Driver/aarch64-cpus.c (original)
> +++ cfe/trunk/test/Driver/aarch64-cpus.c Wed Sep 23 04:29:32 2015
> @@ -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"
>
>
> 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=248370&r1=248369&r2=248370&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)
> +++ cfe/trunk/test/Driver/arm-cortex-cpus.c Wed Sep 23 04:29:32 2015
> @@ -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"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
>
>
> --
>
> Alexander Kornienko | Software Engineer | alexfh at google.com | Google
> Germany, Munich
>
> -- 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.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
> Registered in England & Wales, Company No: 2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
> Registered in England & Wales, Company No: 2548782
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150923/b03c355d/attachment-0001.html>
More information about the cfe-commits
mailing list