This is the mail archive of the
libc-ports@sources.redhat.com
mailing list for the libc-ports project.
Re: [PATCH] sysdeps/arm/armv7/multiarch/memcpy_impl.S: Improve performance.
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Will Newton <will dot newton at linaro dot org>
- Cc: "libc-ports at sourceware dot org" <libc-ports at sourceware dot org>, Patch Tracking <patches at linaro dot org>
- Date: Fri, 30 Aug 2013 15:18:25 +0000
- Subject: Re: [PATCH] sysdeps/arm/armv7/multiarch/memcpy_impl.S: Improve performance.
- Authentication-results: sourceware.org; auth=none
- References: <520894D5 dot 7060207 at linaro dot org> <Pine dot LNX dot 4 dot 64 dot 1308292353450 dot 1487 at digraph dot polyomino dot org dot uk> <CANu=DmgLOGuVi9gjdZ2iVHsPKbH2BWm=ykvJ3qxZ9pDL+H8oxg at mail dot gmail dot 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