[PATCH] ARMv7A Cleanup

[Renato Golin]

On 10 September 2013 22:15, Jim Grosbach <grosbach at apple.com> wrote:

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

Hi Jim, Bob,

Thanks for the review!

Yes, there's a lot going on and this is a medley of everything. I'll split
into multiple patches, but I wasn't sure what's worth and what's not.

Bob, you are right that this is *mostly* a tidy-up without much thought, so
please disregard if there's any change that doesn't make sense.

About the "v7" to "v7a" change, I was just wondering why it was that way,
now I know. This change is not necessary for anything, but the main problem
with being "v7" is that when you're converting architecture strings in the
driver (which happens many times per invocation), the triple changes, and
the default CPU is not the same.

One case, which I fixed by introducing the changed triple to default CPU
detection, is the change from "armv6m" to "thumbv6m".

But I don't have any special case, so we can drop that until it becomes
relevant. When it does, we'll have to take into account Darwin triples, and
that might not be trivial...

Now, on to specific comments...



>
> On Sep 10, 2013, at 1:58 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>
>
> 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?
>

Not important right now, I'll drop it.


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

According to the GCC folks, it's for "little-endian", and it's used by
Linux distributions to compile the kernel. This change is not required by
anything now. The default on Cortex-A8 is because that's the oldest v7 that
runs Linux kernels at the moment.

Again, not important for now.


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.
>
> I'll use "v7" for everything, for now.


So, in the end, there are three changes:
 1. Adding all Cortex-A CPUs to CPU-related functions
 2. Fix NEON+VFP3 for some Cortex-A CPUs
 3. Fix v6M CPU detection

I'll split in three patches and re-send.

Thanks!
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130911/72ec0226/attachment.html>