Re: [PATCH] sysdeps/arm/armv7/multiarch/memcpy_impl.S: Improve performance.

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

On 30 August 2013 16:18, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Fri, 30 Aug 2013, Will Newton wrote:
>
>> > There are various comments regarding alignment, whether stating "LDRD/STRD
>> > support unaligned word accesses" or referring to the mutual alignment that
>> > applies for particular code.  Does this patch make any of them out of
>> > date?  (If code can now only be reached with common 64-bit alignment, but
>> > in fact requires only 32-bit alignment, the comment should probably state
>> > both those things explicitly.)
>>
>> I've reviewed the comments and they all look ok as far as I can tell.
>
> Are you sure?  For example, where it says "SRC and DST have the same
> mutual 32-bit alignment", is it not in fact the case after the patch that
> they now have the same mutual 64-bit alignment, even if this code doesn't
> currently rely on that?  I think the comments in each place should be
> explicit about both things - what preconditions the code relies on, and
> what possibly stronger preconditions are in fact true given the other code
> in this file.  Saying the mutual alignment is 32-bit when the reader can
> see from the code not far above that it's 64-bit just seems liable to
> confuse the reader, even if the comment is still formally true.
> Similarly for the requirements on unaligned word accesses - after this
> patch, which uses of ldrd/strd do require that?

Yes, you're right, that needs more thought. I'll have a look at it next week.

-- 
Will Newton
Toolchain Working Group, Linaro