Re: PowerPC LE memchr and memrchr

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

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.
>