This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: PowerPC LE memcmp
- From: Will Schmidt <will_schmidt at vnet dot ibm dot com>
- To: Alan Modra <amodra at gmail dot com>, "Ryan S. Arnold" <ryan dot arnold at gmail dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 14 Aug 2013 17:17:11 -0500
- Subject: Re: PowerPC LE memcmp
- References: <20130809052322 dot GM3294 at bubble dot grove dot modra dot org>
- Reply-to: will_schmidt at vnet dot ibm dot com
On Fri, 2013-08-09 at 14:53 +0930, Alan Modra wrote:
> This is a rather large patch due to formatting and renaming. The
> formatting changes were to make it possible to compare power7 and
> power4 versions of memcmp. Using different register defines came
> about while I was wrestling with the code, trying to find spare
> registers at one stage. I found it much simpler if we refer to a reg
> by the same name throughout a function, so it's better if short-term
> multiple use regs like rTMP are referred to using their register
> number. I made the cr field usage changes when attempting to reload
> rWORDn regs in the exit path to byte swap before comparing when
> little-endian. That proved a bad idea due to the pipelining involved
> in the main loop; Offsets to reload the regs were different first
> time around the loop.. Anyway, I left the cr field usage changes in
> place for consistency.
>
> Aside from these more-or-less cosmetic changes, I fixed a number of
> places where an early exit path restores regs unnecessarily, removed
> some dead code, and optimised one or two exits.
<...>
> .align 4
> L(bytealigned):
> mtctr rN
> - beq cr6,L(zeroLength)
> +#if 0
> +/* Huh? We've already branched on cr6! */
> + beq cr6, L(zeroLength)
> +#endif
The "We've already branched on cr*" gives me a shiver, but I think in
those cases the earlier branch should have been on a different
conditional. (bgtlr, ... beq ). If not, it's a dead path.
As far as that goes, I think you can go ahead and chop those #if 0
blocks entirely, unless you think there is more investigation to be done
on those.
I'm assuming that these changes have passed testing. :-)
Looks OK to me, thanks,
-Will