[PATCH] [clang] dont special case the ARMv7 FPU

[Jim Grosbach]

On Feb 19, 2013, at 2:32 PM, Saleem Abdulrasool <compnerd at compnerd.org> wrote:

> On Tue, Feb 19, 2013 at 1:59 PM, Jim Grosbach <grosbach at apple.com> wrote:
>> iOS wants to default to a CPU setting of cortex-a8 when compiling for an
>> arch of v7. That implies NEON. I don't know if this particular bit of code
>> is necessary to make that happen properly. I suggest experimenting and
>> digging a bit to see how that works.
> 
> AIUI, ARMv7 does not imply NEON.  You can have systems with a Cortex
> A-8 CPU and not have a NEON FPU.  Of course, this is not something
> that is desirable, but, the implication does not always hold is the
> problem.  Now, special casing this for the iOS platform is perfectly
> acceptable, since in that case, you know that there is a NEON FPU
> available.
> 

As we discussed in IRC, it's not a matter of implication. It's the matter of choosing a reasonable default. NEON is a reasonable default. We would need a strong motivation to change that behavior for everyone currently using LLVM for an armv7a device. I'm sorry, but the existence of tegra1/2 is not a compelling motivator.

-Jim


>> In any case, changing the default of whether NEON is available for armv7
>> when compiling for iOS is very much the wrong thing to do.
>> 
>> -Jim
>> 
>> 
>> On Feb 19, 2013, at 1:53 PM, Eric Christopher <echristo at gmail.com> wrote:
>> 
>> At the very least I agree that's the wrong place to be doing that. Or it
>> should be conditionalized on OS if the ios people want to do that.
>> 
>> -eric
>> 
>> On Tue, Feb 19, 2013 at 1:50 PM, Saleem Abdulrasool <compnerd at compnerd.org>
>> wrote:
>>> 
>>> Hi echristo, grosbach,
>>> 
>>> ARMv7 chipsets usually are paired with a NEON FPU.  However, this is not
>>> required (e.g. tegra2).  Rather than always forcing NEON as the FPU for
>>> ARMv7
>>> chips, honour the -mfpu flag, which already is being done.  If -mfpu is
>>> not
>>> specified, then let the default FPU remain selected (VFP).
>>> 
>>> Signed-off-by: Saleem Abdulrasool <compnerd at compnerd.org>
>>> 
>>> 
>>> http://llvm-reviews.chandlerc.com/D425
>>> 
>>> Files:
>>>  lib/Driver/Tools.cpp
>>> 
>>> Index: lib/Driver/Tools.cpp
>>> ===================================================================
>>> --- lib/Driver/Tools.cpp
>>> +++ lib/Driver/Tools.cpp
>>> @@ -5494,10 +5494,6 @@
>>>     CmdArgs.push_back("-mppc64");
>>>     CmdArgs.push_back("-many");
>>>   } else if (getToolChain().getArch() == llvm::Triple::arm) {
>>> -    StringRef MArch = getToolChain().getArchName();
>>> -    if (MArch == "armv7" || MArch == "armv7a" || MArch == "armv7-a")
>>> -      CmdArgs.push_back("-mfpu=neon");
>>> -
>>>     StringRef ARMFloatABI = getARMFloatABI(getToolChain().getDriver(),
>>> Args,
>>>                                            getToolChain().getTriple());
>>>     CmdArgs.push_back(Args.MakeArgString("-mfloat-abi=" + ARMFloatABI));
>> 
>> 
>> 
> 
> 
> 
> -- 
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org