This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: PowerPC LE memchr and memrchr
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: libc-alpha at sourceware dot org
- Date: Wed, 28 Aug 2013 09:49:21 -0300
- Subject: Re: PowerPC LE memchr and memrchr
- Authentication-results: sourceware.org; auth=none
- References: <20130809053000 dot GP3294 at bubble dot grove dot modra dot org> <1376494372 dot 3823 dot 28 dot camel at brimstone> <20130815073629 dot GD4024 at bubble dot grove dot modra dot org>
Hi Alan, this patch is ok. I also found out a issue with current memrchr
implementation for POWER7 that your patch also corrects (a wrong pointer
passed to loop_small branch):
--- a/sysdeps/powerpc/powerpc32/power7/memrchr.S
+++ b/sysdeps/powerpc/powerpc32/power7/memrchr.S
@@ -101,8 +101,8 @@ L(loop):
/* We're here because the counter reached 0, and that means we
didn't have any matches for BYTE in the whole range. Just return
the original range. */
- addi r9,r8,4
- cmplw cr6,r9,r7
+ addi r8,r8,4
+ cmplw cr6,r8,r7
bgt cr6,L(loop_small)
b L(null)
On 08/15/2013 04:36 AM, Alan Modra wrote:
> On Wed, Aug 14, 2013 at 10:32:52AM -0500, Will Schmidt wrote:
>> ".p2align alignment" versus ".align alignment" - Cosmetic or deliberate
>> to avoid some unintended .p2align alignment[,fill[,max]] behavior?
> Cosmetic. .align and .p2align are identical on powerpc.
>
>> Looks like we initially have a mix of the two, so would be good to be
>> clean those up regardless.
> Agreed.
>
>>> - b L(loop)
>>> - /* Main loop to look for BYTE backwards in the string. Since it's a
>>> - small loop (< 8 instructions), align it to 32-bytes. */
>>> - .p2align 5
>>> +
>>> + /* Main loop to look for BYTE backwards in the string. */
>>> + .align 4
>> deliberate switch from 5 to 4 ?
> Good question. I was experimenting, and forgot to remove the
> experiment.. The thing is that the loop following does not have less
> that eight instructions (it has nine), so there's no rationale for
> aligning to 32 bytes. If you believe the comment, that is. And I
> guess the comment really ought to have said "<= 8 instructions".
>
> I'll remove that change.
>