Re: [PATCH 1/4] [Powerpc] tune/optimize memmove/wordcopy. Add helpermacro to call MERGE

["Ryan S. Arnold" <ryan dot arnold at gmail dot com>]

On Mon, Mar 19, 2012 at 4:01 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote:
> [Powerpc] tune/optimize memmove/wordcopy. Add helper macro to call MERGE
>
> Add helper macros to handle calling the MERGE function with the proper
> alignments. ÂThis significantly shortens the code.
>
> 2012-03-19 ÂWill Schmidt <will_schmidt@vnet.ibm.com>
>
> Â Â Â Â* sysdeps/powerpc/powerpc64/power6/wordcopy.c: Add fwd_align_merge and
> Â Â Â Âbwd_align_merge macros.
> Â Â Â Â* sysdeps/powerpc/powerpc32/power6/wordcopy.c: Likewise.
> ---
> Âsysdeps/powerpc/powerpc32/power6/wordcopy.c | Â117 ++++----------
> Âsysdeps/powerpc/powerpc64/power6/wordcopy.c | Â234 +++++----------------------
> Â2 files changed, 79 insertions(+), 272 deletions(-)
>
> diff --git a/sysdeps/powerpc/powerpc32/power6/wordcopy.c b/sysdeps/powerpc/powerpc32/power6/wordcopy.c
> index ddf2865..9748268 100644
> --- a/sysdeps/powerpc/powerpc32/power6/wordcopy.c
> +++ b/sysdeps/powerpc/powerpc32/power6/wordcopy.c
> @@ -1,5 +1,5 @@
> Â/* _memcopy.c -- subroutines for memory copy functions.
> - Â Copyright (C) 1991, 1996, 2006 Free Software Foundation, Inc.
> + Â Copyright (C) 1991, 1996, 2006, 2012 Free Software Foundation, Inc.

Per the contribution checklist you may now collapse the copyright year
range, e.g.,

Copyright (C) 1991-2012 Free Software Foundation, Inc.

> Â ÂThis file is part of the GNU C Library.
> Â ÂContributed by Torbjorn Granlund (tege@sics.se).
> Â ÂUpdated for POWER6 by Steven Munroe (sjmunroe@us.ibm.com).
> @@ -66,6 +66,20 @@ _wordcopy_fwd_aligned (dstp, srcp, len)
> Â ÂDSTP should be aligned for memory operations on `op_t's, but SRCP must
> Â Â*not* be aligned. Â*/
>
> +#define fwd_align_merge(align) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Âdo                                  \
> + Â Â{ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â Âa1 = ((op_t *) srcp)[1]; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Âa2 = ((op_t *) srcp)[2]; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â((op_t *) dstp)[0] = MERGE (a0, align*8, a1, (32-align*8)); Â Â Â\
> + Â Â Â((op_t *) dstp)[1] = MERGE (a1, align*8, a2, (32-align*8)); Â Â Â\
> + Â Â Âa0 = a2; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Âsrcp += 2 * OPSIZ; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Âdstp += 2 * OPSIZ; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Âlen -= 2; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â} Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Âwhile (len != 0);
> +
> Âvoid
> Â_wordcopy_fwd_dest_aligned (dstp, srcp, len)
> Â Â Âlong int dstp;
> @@ -105,49 +119,13 @@ _wordcopy_fwd_dest_aligned (dstp, srcp, len)
> Â switch (align)
> Â Â {
> Â Â case 1:
> - Â Â Âdo
> - Â Â Â Â{
> - Â Â Â Â Âa1 = ((op_t *) srcp)[1];
> - Â Â Â Â Âa2 = ((op_t *) srcp)[2];
> - Â Â Â Â Â((op_t *) dstp)[0] = MERGE (a0, 8, a1, (32-8));
> - Â Â Â Â Â((op_t *) dstp)[1] = MERGE (a1, 8, a2, (32-8));
> - Â Â Â Â Âa0 = a2;
> -
> - Â Â Â Â Âsrcp += 2 * OPSIZ;
> - Â Â Â Â Âdstp += 2 * OPSIZ;
> - Â Â Â Â Âlen -= 2;
> - Â Â Â Â}
> - Â Â Âwhile (len != 0);
> + Â Â Âfwd_align_merge(1)

This should be and future instances of it and bwd_align_merge should
have a space between then symbol name and the parenthesis, e.g.,

fwd_align_merge (1)

I know that technically due to CPP macro expansion that a trailing
semi-colon is not necessary, but is it preferred?

fwd_align_merge (1);

Ryan