Segher Boessenkool <seg...@kernel.crashing.org> writes:
> On Fri, Mar 22, 2019 at 11:37:24PM +1100, Michael Ellerman wrote:
>> .Lcmp_rest_lt8bytes:
>> - /* Here we have only less than 8 bytes to compare with. at least s1
>> - * Address is aligned with 8 bytes.
>> - * The next double words are load and shift right with appropriate
>> - * bits.
>> + /*
>> + * Here we have less than 8 bytes to compare. At least s1 is aligned to
>> + * 8 bytes, but s2 may not be. We must make sure s2 + 8 doesn't cross a
>
> "s2 + 7"? The code is fine though (bgt, not bge).
Duh, thanks for catching it.
>> + * page boundary, otherwise we might read past the end of the buffer and
>> + * trigger a page fault. We use 4K as the conservative minimum page
>> + * size. If we detect that case we go to the byte-by-byte loop.
>> + *
>> + * Otherwise the next double word is loaded from s1 and s2, and shifted
>> + * right to compare the appropriate bits.
>> */
>> + clrldi r6,r4,(64-12) // r6 = r4 & 0xfff
>
> You can just write
> rlwinm r6,r4,0,0x0fff
> if that is clearer? Or do you still want a comment with that :-)
I don't think it's clearer doing a rotate of zero bits :)
And yeah I'd probably still leave the comment, so I'm inclined to stick
with the clrldi?
Would be nice if the assembler could support:
andi r6, r4, 0x0fff
And turn it into the rlwinm, or rldicl :)
>> + cmpdi r6,0xff8
>> + bgt .Lshort
>
> Reviewed-by: Segher Boessenkool <seg...@kernel.crashing.org>
I'll fixup the comment. Thanks.
cheers
