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

["Joseph S. Myers" <joseph@codesourcery.com>]

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?

-- 
Joseph S. Myers
joseph@codesourcery.com