[PATCH] ARMv7A Cleanup

[Jim Grosbach]

If nothing else, this strikes me as something that should really be multiple district patches. There’s multiple things going on here and I’m not sure what’s what.

On Sep 10, 2013, at 1:58 PM, Bob Wilson <bob.wilson at apple.com> wrote:

> [ adding Jim Grosbach, who may have input on the architectures used in triples ]
> 
> On Sep 3, 2013, at 4:56 AM, Renato Golin <renato.golin at linaro.org> wrote:
> 
>> Hi doug.gregor,
>> 
>> This is a cleanup of the ARMv7A CPUs and their options and features.
>> 
>> - Adding Cortex-A12 and A5 and making sure all cores are in all lists. (I'm not sure I got them all).
>> - Changing "v7" to "v7a", "7" to "7A" on v7A CPUs.
>> - Forced "vfp3" to A8/A9, added "vfp4" for A15 and fixed the NEON feature check
>> - Adding "armv7l" Arch, that seems popular with Linux distros
>> 
>> I'm not sure how to add tests for all these things, pointers welcome!
>> 
>> http://llvm-reviews.chandlerc.com/D1579
>> 
>> Files:
>> lib/Basic/Targets.cpp
>> lib/Driver/ToolChain.cpp
>> lib/Driver/ToolChains.cpp
>> lib/Driver/Tools.cpp
>> test/Driver/arch.c
>> 
>> Index: lib/Driver/ToolChain.cpp
>> ===================================================================
>> --- lib/Driver/ToolChain.cpp
>> +++ lib/Driver/ToolChain.cpp
>> @@ -237,8 +237,8 @@
>>    .Cases("arm1136j-s",  "arm1136jf-s",  "arm1176jz-s", "v6")
>>    .Cases("arm1176jzf-s",  "mpcorenovfp",  "mpcore", "v6")
>>    .Cases("arm1156t2-s",  "arm1156t2f-s", "v6t2")
>> -    .Cases("cortex-a5", "cortex-a7", "cortex-a8", "v7")
>> -    .Cases("cortex-a9", "cortex-a15", "v7")
>> +    .Cases("cortex-a5", "cortex-a7", "cortex-a8", "v7a")
>> +    .Cases("cortex-a9", "cortex-a12", "cortex-a15", "v7a")
> 
> This is going to change the triples to use "armv7a" instead of "armv7".  What have you done to test that?  How important is it to make that change?  We could just continue to use "armv7" in place of "armv7a”.

+1. I don’t follow why this is a good change to make. If it’s just nomenclature tidy-up, I really don’t think it’s worth it. Is there more too it than that?

> 
>>    .Case("cortex-r5", "v7r")
>>    .Case("cortex-m0", "v6m")
>>    .Case("cortex-m3", "v7m")
>> Index: lib/Driver/ToolChains.cpp
>> ===================================================================
>> --- lib/Driver/ToolChains.cpp
>> +++ lib/Driver/ToolChains.cpp
>> @@ -112,6 +112,7 @@
>>    .Cases("armv7k", "armv7-k", "armv7k")
>>    .Cases("armv7m", "armv7-m", "armv7m")
>>    .Cases("armv7s", "armv7-s", "armv7s")
>> +    .Cases("armv7l", "armv7-l", "armv7l")
> 
> What is armv7l for?  I see one place in lib/Driver/ToolChain.cpp where it is mapped to a cortex-a8 processor, but that doesn't make sense to me.
> 

First I’ve ever heard of it, too. I have no philosophical objection, really, but would like a bit more clarity on what it actually is if that’s possible.

>>    .Cases("armv8", "armv8a", "armv8-a", "armv8")
>>    .Default(0);
>> }
>> @@ -124,7 +125,9 @@
>>    .Case("xscale", "xscale")
>>    .Cases("arm1136j-s", "arm1136jf-s", "arm1176jz-s", "arm1176jzf-s", "armv6")
>>    .Case("cortex-m0", "armv6m")
>> -    .Cases("cortex-a8", "cortex-r4", "cortex-a9", "cortex-a15", "armv7")
>> +    .Cases("cortex-a5", "cortex-a7", "cortex-a8", "armv7a")
>> +    .Cases("cortex-a9", "cortex-a12", "cortex-a15", "armv7a")
> 
> This will break stuff.  It is only used by Darwin::getDarwinArchName, and Darwin does not ever expect to see an "armv7a" architecture.

+1

>> +    .Case("cortex-r4", "armv7r")
>>    .Case("cortex-a9-mp", "armv7f")
>>    .Case("cortex-m3", "armv7m")
>>    .Case("cortex-m4", "armv7em")
>> Index: lib/Driver/Tools.cpp
>> ===================================================================
>> --- lib/Driver/Tools.cpp
>> +++ lib/Driver/Tools.cpp
>> @@ -458,8 +458,8 @@
>>    .Cases("arm1136j-s",  "arm1136jf-s",  "arm1176jz-s", "v6")
>>    .Cases("arm1176jzf-s",  "mpcorenovfp",  "mpcore", "v6")
>>    .Cases("arm1156t2-s",  "arm1156t2f-s", "v6t2")
>> -    .Cases("cortex-a5", "cortex-a7", "cortex-a8", "v7")
>> -    .Cases("cortex-a9", "cortex-a15", "v7")
>> +    .Cases("cortex-a5", "cortex-a7", "cortex-a8", "v7a")
>> +    .Cases("cortex-a9", "cortex-a12", "cortex-a15", "v7a")
> 
> Aside from adding cortex-a12, this shouldn't change anything.  The result of that function is either only compared to the "v7" prefix or handled in the StringSwitch at the end of getARMTargetCPU, where "v7" and "v7a" are treated the same.
> 
>>    .Case("cortex-r5", "v7r")
>>    .Case("cortex-m0", "v6m")
>>    .Case("cortex-m3", "v7m")
>> Index: test/Driver/arch.c
>> ===================================================================
>> --- test/Driver/arch.c
>> +++ test/Driver/arch.c
>> @@ -1,5 +1,5 @@
>> // RUN: %clang -target armv7a-unknown-linux-gnueabi -S -emit-llvm %s -o - | FileCheck %s --check-prefix=V7
>> // RUN: %clang -target armv8a-unknown-linux-gnueabi -S -emit-llvm %s -o - | FileCheck %s --check-prefix=V8
>> 

The backend tools (llc, et. al.) also make the “v7 == v7a” triple simplification. If that’s changing in the front end, it should change in the backend too.


>> -// V7: target triple = "armv7-unknown-linux-gnueabi"
>> +// V7: target triple = "armv7a-unknown-linux-gnueabi"
>> // V8: target triple = "armv8-unknown-linux-gnueabi"
>> <D1579.1.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130910/a5c7306a/attachment.html>