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

[Evgeniy Stepanov]

On Wed, Jan 11, 2012 at 3:37 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 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).

AFAIK it does not change. I've added tests for the default case, and
for the special case of -msoft-float (which disables Neon).

>
>>
>> +// 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.

Sorry, that was a stale .subversion/config.