[v6,1/4] powerpc/64: Align bytes before fall back to .Lshort in powerpc64 memcmp()

From: Simon Guo <wei.guo.simon@gmail.com>

Currently memcmp() 64bytes version in powerpc will fall back to .Lshort
(compare per byte mode) if either src or dst address is not 8 bytes aligned.
It can be opmitized in 2 situations:

1) if both addresses are with the same offset with 8 bytes boundary:
memcmp() can compare the unaligned bytes within 8 bytes boundary firstly
and then compare the rest 8-bytes-aligned content with .Llong mode.

2)  If src/dst addrs are not with the same offset of 8 bytes boundary:
memcmp() can align src addr with 8 bytes, increment dst addr accordingly,
 then load src with aligned mode and load dst with unaligned mode.

This patch optmizes memcmp() behavior in the above 2 situations.

Tested with both little/big endian. Performance result below is based on
little endian.

Following is the test result with src/dst having the same offset case:
(a similar result was observed when src/dst having different offset):
(1) 256 bytes
Test with the existing tools/testing/selftests/powerpc/stringloops/memcmp:
- without patch
	29.773018302 seconds time elapsed                                          ( +- 0.09% )
- with patch
	16.485568173 seconds time elapsed                                          ( +-  0.02% )
		-> There is ~+80% percent improvement

(2) 32 bytes
To observe performance impact on < 32 bytes, modify
tools/testing/selftests/powerpc/stringloops/memcmp.c with following:
-------
 #include <string.h>
 #include "utils.h"

-#define SIZE 256
+#define SIZE 32
 #define ITERATIONS 10000

 int test_memcmp(const void *s1, const void *s2, size_t n);
--------

- Without patch
	0.244746482 seconds time elapsed                                          ( +-  0.36%)
- with patch
	0.215069477 seconds time elapsed                                          ( +-  0.51%)
		-> There is ～+13% improvement

(3) 0~8 bytes
To observe <8 bytes performance impact, modify
tools/testing/selftests/powerpc/stringloops/memcmp.c with following:
-------
 #include <string.h>
 #include "utils.h"

-#define SIZE 256
-#define ITERATIONS 10000
+#define SIZE 8
+#define ITERATIONS 1000000

 int test_memcmp(const void *s1, const void *s2, size_t n);
-------
- Without patch
       1.845642503 seconds time elapsed                                          ( +- 0.12% )
- With patch
       1.849767135 seconds time elapsed                                          ( +- 0.26% )
		-> They are nearly the same. (-0.2%)

Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
 arch/powerpc/lib/memcmp_64.S | 143 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 136 insertions(+), 7 deletions(-)

On Fri, May 25, 2018 at 12:07:33PM +0800, wei.guo.simon@gmail.com wrote:
>  _GLOBAL(memcmp)
>  	cmpdi	cr1,r5,0
>  
> -	/* Use the short loop if both strings are not 8B aligned */
> -	or	r6,r3,r4
> +	/* Use the short loop if the src/dst addresses are not
> +	 * with the same offset of 8 bytes align boundary.
> +	 */
> +	xor	r6,r3,r4
>  	andi.	r6,r6,7
>  
> -	/* Use the short loop if length is less than 32B */
> -	cmpdi	cr6,r5,31
> +	/* Fall back to short loop if compare at aligned addrs
> +	 * with less than 8 bytes.
> +	 */
> +	cmpdi   cr6,r5,7
>  
>  	beq	cr1,.Lzero
> -	bne	.Lshort
> -	bgt	cr6,.Llong
> +	bgt	cr6,.Lno_short

If this doesn't use cr0 anymore, you can do  rlwinm r6,r6,0,7  instead of
andi r6,r6,7 .

> +.Lsameoffset_8bytes_make_align_start:
> +	/* attempt to compare bytes not aligned with 8 bytes so that
> +	 * rest comparison can run based on 8 bytes alignment.
> +	 */
> +	andi.   r6,r3,7
> +
> +	/* Try to compare the first double word which is not 8 bytes aligned:
> +	 * load the first double word at (src & ~7UL) and shift left appropriate
> +	 * bits before comparision.
> +	 */
> +	clrlwi  r6,r3,29
> +	rlwinm  r6,r6,3,0,28

Those last two lines are together just
  rlwinm r6,r3,3,0x1c

> +	subfc.	r5,r6,r5

Why subfc?  You don't use the carry.

> +	rlwinm  r6,r6,3,0,28

That's
  slwi r6,r6,3

> +	bgt	cr0,8f
> +	li	r3,-1
> +8:
> +	blr

  blelr
  li r3,-1
  blr

(and more of the same things elsewhere).


Segher

Hi Segher,
On Mon, May 28, 2018 at 05:35:12AM -0500, Segher Boessenkool wrote:
> On Fri, May 25, 2018 at 12:07:33PM +0800, wei.guo.simon@gmail.com wrote:
> >  _GLOBAL(memcmp)
> >  	cmpdi	cr1,r5,0
> >  
> > -	/* Use the short loop if both strings are not 8B aligned */
> > -	or	r6,r3,r4
> > +	/* Use the short loop if the src/dst addresses are not
> > +	 * with the same offset of 8 bytes align boundary.
> > +	 */
> > +	xor	r6,r3,r4
> >  	andi.	r6,r6,7
> >  
> > -	/* Use the short loop if length is less than 32B */
> > -	cmpdi	cr6,r5,31
> > +	/* Fall back to short loop if compare at aligned addrs
> > +	 * with less than 8 bytes.
> > +	 */
> > +	cmpdi   cr6,r5,7
> >  
> >  	beq	cr1,.Lzero
> > -	bne	.Lshort
> > -	bgt	cr6,.Llong
> > +	bgt	cr6,.Lno_short
> 
> If this doesn't use cr0 anymore, you can do  rlwinm r6,r6,0,7  instead of
> andi r6,r6,7 .
> 
CR0 is used at .Lno_short handling.

> > +.Lsameoffset_8bytes_make_align_start:
> > +	/* attempt to compare bytes not aligned with 8 bytes so that
> > +	 * rest comparison can run based on 8 bytes alignment.
> > +	 */
> > +	andi.   r6,r3,7
> > +
> > +	/* Try to compare the first double word which is not 8 bytes aligned:
> > +	 * load the first double word at (src & ~7UL) and shift left appropriate
> > +	 * bits before comparision.
> > +	 */
> > +	clrlwi  r6,r3,29
> > +	rlwinm  r6,r6,3,0,28
> 
> Those last two lines are together just
>   rlwinm r6,r3,3,0x1c
> 
Yes. I will combine them.

> > +	subfc.	r5,r6,r5
> 
> Why subfc?  You don't use the carry.
OK. I will use subfc instead.

> 
> > +	rlwinm  r6,r6,3,0,28
> 
> That's
>   slwi r6,r6,3
Yes.

> 
> > +	bgt	cr0,8f
> > +	li	r3,-1
> > +8:
> > +	blr
> 
>   blelr
>   li r3,-1
>   blr
Sure. That looks more impact.

> 
> (and more of the same things elsewhere).
> 
> 
> Segher

Thanks for your good comments.

BR,
- Simon

Hi!

On Wed, May 30, 2018 at 04:11:50PM +0800, Simon Guo wrote:
> On Mon, May 28, 2018 at 05:35:12AM -0500, Segher Boessenkool wrote:
> > On Fri, May 25, 2018 at 12:07:33PM +0800, wei.guo.simon@gmail.com wrote:
> > If this doesn't use cr0 anymore, you can do  rlwinm r6,r6,0,7  instead of
> > andi r6,r6,7 .
> > 
> CR0 is used at .Lno_short handling.

Tricky.

> > > +	subfc.	r5,r6,r5
> > 
> > Why subfc?  You don't use the carry.
> OK. I will use subfc instead.

I meant subf -- no carry.  If you want CR0 set there is subf. just fine.

> > > +	bgt	cr0,8f
> > > +	li	r3,-1
> > > +8:
> > > +	blr
> > 
> >   blelr
> >   li r3,-1
> >   blr
> Sure. That looks more impact.

Should have been bgtlr of course -- well check please :-)


Segher

On Wed, May 30, 2018 at 03:27:39AM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, May 30, 2018 at 04:11:50PM +0800, Simon Guo wrote:
> > On Mon, May 28, 2018 at 05:35:12AM -0500, Segher Boessenkool wrote:
> > > On Fri, May 25, 2018 at 12:07:33PM +0800, wei.guo.simon@gmail.com wrote:
> > > If this doesn't use cr0 anymore, you can do  rlwinm r6,r6,0,7  instead of
> > > andi r6,r6,7 .
> > > 
> > CR0 is used at .Lno_short handling.
> 
> Tricky.
> 
> > > > +	subfc.	r5,r6,r5
> > > 
> > > Why subfc?  You don't use the carry.
> > OK. I will use subfc instead.
> 
> I meant subf -- no carry.  If you want CR0 set there is subf. just fine.
> 
> > > > +	bgt	cr0,8f
> > > > +	li	r3,-1
> > > > +8:
> > > > +	blr
> > > 
> > >   blelr
> > >   li r3,-1
> > >   blr
> > Sure. That looks more impact.
> 
> Should have been bgtlr of course -- well check please :-)
Yes :)

Thanks,
- Simon