[cfe-commits] r147943 - in /cfe/trunk: lib/Basic/Targets.cpp lib/Driver/Tools.cpp test/Driver/arm-mfpu.c

[Chandler Carruth]

A few follow-up comments in-line....

On Wed, Jan 11, 2012 at 3:21 AM, Evgeniy Stepanov <eugeni.stepanov at gmail.com
> wrote:

> Author: eugenis
> Date: Wed Jan 11 05:21:31 2012
> New Revision: 147943
>
> URL: http://llvm.org/viewvc/llvm-project?rev=147943&view=rev
> Log:
> Fix -mfpu parsing on ARM.
>
> - Support gcc-compatible vfpv3 name in addition to vfp3.
> - Support vfpv3-d16.
> - Disable neon feature for -mfpu=vfp* (yes, we were emitting Neon
> instructions
>  for those!).
>
>
> Added:
>    cfe/trunk/test/Driver/arm-mfpu.c   (with props)
> Modified:
>    cfe/trunk/lib/Basic/Targets.cpp
>    cfe/trunk/lib/Driver/Tools.cpp
>
> Modified: cfe/trunk/lib/Basic/Targets.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=147943&r1=147942&r2=147943&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> +++ cfe/trunk/lib/Basic/Targets.cpp Wed Jan 11 05:21:31 2012
> @@ -2658,7 +2658,7 @@
>                                  const std::string &Name,
>                                  bool Enabled) const {
>     if (Name == "soft-float" || Name == "soft-float-abi" ||
> -        Name == "vfp2" || Name == "vfp3" || Name == "neon") {
> +        Name == "vfp2" || Name == "vfp3" || Name == "neon" || Name ==
> "d16") {
>       Features[Name] = Enabled;
>     } else
>       return false;
>
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=147943&r1=147942&r2=147943&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Wed Jan 11 05:21:31 2012
> @@ -660,12 +660,23 @@
>       CmdArgs.push_back("-vfp3");
>       CmdArgs.push_back("-target-feature");
>       CmdArgs.push_back("-neon");
> +    } else if (FPU == "vfp3-d16" || FPU == "vfpv3-d16") {
> +      CmdArgs.push_back("-target-feature");
> +      CmdArgs.push_back("+vfp3");
> +      CmdArgs.push_back("-target-feature");
> +      CmdArgs.push_back("+d16");
> +      CmdArgs.push_back("-target-feature");
> +      CmdArgs.push_back("-neon");
>     } else if (FPU == "vfp") {
>       CmdArgs.push_back("-target-feature");
>       CmdArgs.push_back("+vfp2");
> -    } else if (FPU == "vfp3") {
> +      CmdArgs.push_back("-target-feature");
> +      CmdArgs.push_back("-neon");
> +    } else if (FPU == "vfp3" || FPU == "vfpv3") {
>       CmdArgs.push_back("-target-feature");
>       CmdArgs.push_back("+vfp3");
> +      CmdArgs.push_back("-target-feature");
> +      CmdArgs.push_back("-neon");
>     } else if (FPU == "neon") {
>       CmdArgs.push_back("-target-feature");
>       CmdArgs.push_back("+neon");
>
> Added: cfe/trunk/test/Driver/arm-mfpu.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-mfpu.c?rev=147943&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Driver/arm-mfpu.c (added)
> +++ cfe/trunk/test/Driver/arm-mfpu.c Wed Jan 11 05:21:31 2012
> @@ -0,0 +1,36 @@
>

I really like having a brief note about what is expected to be tested in
this test file at the top with the comments. It helps readers orient
themselves.

Also, you've got great tests for each argument to '-mfpu', but it would be
good to add tests for the default, and the default with different
variations of the arm triple (if that default changes for any variations).


> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=fpa %s -### -o %t.o
> 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-FPA %s
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=fpe2 %s -### -o %t.o
> 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-FPA %s
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=fpe3 %s -### -o %t.o
> 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-FPA %s
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=maverick %s -### -o
> %t.o 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-FPA %s
> +// CHECK-FPA: "-target-feature" "-vfp2"
> +// CHECK-FPA: "-target-feature" "-vfp3"
> +// CHECK-FPA: "-target-feature" "-neon"
>

Thank you for using FileCheck so well! =D


> +
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=vfp3-d16 %s -### -o
> %t.o 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-VFP3-D16 %s
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=vfpv3-d16 %s -### -o
> %t.o 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-VFP3-D16 %s
> +// CHECK-VFP3-D16: "-target-feature" "+vfp3"
> +// CHECK-VFP3-D16: "-target-feature" "+d16"
> +// CHECK-VFP3-D16: "-target-feature" "-neon"
> +
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=vfp %s -### -o %t.o
> 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-VFP %s
> +// CHECK-VFP: "-target-feature" "+vfp2"
> +// CHECK-VFP: "-target-feature" "-neon"
> +
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=vfp3 %s -### -o %t.o
> 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-VFP3 %s
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=vfpv3 %s -### -o
> %t.o 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-VFP3 %s
> +// CHECK-VFP3: "-target-feature" "+vfp3"
> +// CHECK-VFP3: "-target-feature" "-neon"
> +
> +// RUN: %clang -ccc-host-triple arm-linux-eabi -mfpu=neon %s -### -o %t.o
> 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-NEON %s
> +// CHECK-NEON: "-target-feature" "+neon"
> +
>
> Propchange: cfe/trunk/test/Driver/arm-mfpu.c
>
> ------------------------------------------------------------------------------
>    svn:eol-style = LF
>

Ew, can you not fix EOL styles here? your subversion client may well be
misconfigured if its doing this to you by default. I would expect the EOL
style to be 'native' which is supposed to be the default, and causes the
subversion tools to translate the line endings for the platform onto which
they are checking out the source code.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120111/b13ff955/attachment.html>