Re: [PATCH v2] ARM: Add Cortex-A15 optimized NEON and VFP memcpy routines, with IFUNC.

[Will Newton <will.newton@linaro.org>]

On 17 April 2013 16:40, Richard Henderson <rth@twiddle.net> wrote:

Hi Richard,

Thanks for the review!

> On 2013-04-16 11:25, Will Newton wrote:
>>
>>   ports/sysdeps/arm/armv7/multiarch/Makefile         |   3 +
>
>
> Does this really require v7?  From a brief read I didn't see anything in the
> _arm version that didn't work since v5te (ldrd and pld).  Any reason not to
> put this into armv6 instead?

From reading the comments of the code v7 is required for NEON, v6 is
required for VFP and unaligned access is required. The unaligned
access requirement may be a problem on v5 I'm not sure. NB: I did not
write the memcpy code so I have not looked at it in great detail.

I also had trouble building an armv6 glibc. I only have armv7 systems
to test on and it doesn't seem possible to build for armv6 on an armv7
system as far as I can tell.

>> +ENTRY(memcpy)
>> +       .type   memcpy, %gnu_indirect_function
>> +       ldr     r1, .Lmemcpy_arm
>> +       tst     r0, #HWCAP_ARM_NEON
>> +       it      ne
>> +       ldrne   r1, .Lmemcpy_neon
>> +       bne     1f
>
>
> Swap vfp and neon tests and you don't need the branch.

True, I'll do that.

>> +.Lreturn:
>
>
> Unused label?

Yes, thanks, will fix.

>> +       ldr     tmp1, [src, #-60]       /* 15 words to go.  */
>> +       str     tmp1, [dst, #-60]
>
>
> These negative offsets mean thumb2 doesn't work.  That's fine, but it means
> that you need care for this in the _arm case.
>
> You have two choices: either do the swapping to arm mode by hand in the impl
> file, or force the entire memcpy.o to arm mode by using #define NO_THUMB at
> the top, before the #include <sysdep.h>.

It sounds like switching it all to arm mode is the best option, I'll do that.

--
Will Newton
Toolchain Working Group, Linaro