[PATCH v2 2/2] powerpc32: add support for csum_add()

[PATCH v2 2/2] powerpc32: add support for csum_add()

[Christophe Leroy]

The C version of csum_add() as defined in include/net/checksum.h gives the
following assembly:
       0:       7c 04 1a 14     add     r0,r4,r3
       4:       7c 64 00 10     subfc   r3,r4,r0
       8:       7c 63 19 10     subfe   r3,r3,r3
       c:       7c 63 00 50     subf    r3,r3,r0

include/net/checksum.h also offers the possibility to define an arch specific
function.
This patch provides a ppc32 specific csum_add() inline function.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
v2: changed constraints on the __asm__

 arch/powerpc/include/asm/checksum.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h
index cfe806a..1e48cc7 100644
--- a/arch/powerpc/include/asm/checksum.h
+++ b/arch/powerpc/include/asm/checksum.h
@@ -141,6 +141,18 @@ static inline __sum16 csum_tcpudp_magic(__be32 saddr, __be32 daddr,
 {
 	return csum_fold(csum_tcpudp_nofold(saddr, daddr, len, proto, sum));
 }
+
+#define HAVE_ARCH_CSUM_ADD
+static inline __wsum csum_add(__wsum csum, __wsum addend)
+{
+    __asm__("\n\
+	addc %0,%0,%1 \n\
+	addze %0,%0 \n\
+	"
+	: "+r" (csum) : "r" (addend));
+	return csum;
+}
+
 #endif
 #endif
 #endif /* __KERNEL__ */
-- 
2.1.0

Re: [v2,2/2] powerpc32: add support for csum_add()

[Scott Wood]

On Tue, Feb 03, 2015 at 12:39:27PM +0100, LEROY Christophe wrote:
> The C version of csum_add() as defined in include/net/checksum.h gives the
> following assembly:
>        0:       7c 04 1a 14     add     r0,r4,r3
>        4:       7c 64 00 10     subfc   r3,r4,r0
>        8:       7c 63 19 10     subfe   r3,r3,r3
>        c:       7c 63 00 50     subf    r3,r3,r0
> 
> include/net/checksum.h also offers the possibility to define an arch specific
> function.
> This patch provides a ppc32 specific csum_add() inline function.

What makes it 32-bit specific?

-Scott

Re: [v2,2/2] powerpc32: add support for csum_add()

[Scott Wood]

On Tue, Feb 03, 2015 at 12:39:27PM +0100, LEROY Christophe wrote:
> The C version of csum_add() as defined in include/net/checksum.h gives the
> following assembly:
>        0:       7c 04 1a 14     add     r0,r4,r3
>        4:       7c 64 00 10     subfc   r3,r4,r0
>        8:       7c 63 19 10     subfe   r3,r3,r3
>        c:       7c 63 00 50     subf    r3,r3,r0
> 
> include/net/checksum.h also offers the possibility to define an arch specific
> function.
> This patch provides a ppc32 specific csum_add() inline function.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> v2: changed constraints on the __asm__
> 
>  arch/powerpc/include/asm/checksum.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h
> index cfe806a..1e48cc7 100644
> --- a/arch/powerpc/include/asm/checksum.h
> +++ b/arch/powerpc/include/asm/checksum.h
> @@ -141,6 +141,18 @@ static inline __sum16 csum_tcpudp_magic(__be32 saddr, __be32 daddr,
>  {
>  	return csum_fold(csum_tcpudp_nofold(saddr, daddr, len, proto, sum));
>  }
> +
> +#define HAVE_ARCH_CSUM_ADD
> +static inline __wsum csum_add(__wsum csum, __wsum addend)
> +{
> +    __asm__("\n\

s/__asm__/asm/

Use tabs to indent

> +	addc %0,%0,%1 \n\
> +	addze %0,%0 \n\

Use ; to separate asm statements instead of using \n
Use string concatenation instead of \

-Scott

Re: [v2,2/2] powerpc32: add support for csum_add()

[Scott Wood]

On Tue, 2015-04-28 at 21:01 +0200, christophe leroy wrote:
> 
> 
> Le 25/03/2015 02:30, Scott Wood a écrit :
> 
> > On Tue, Feb 03, 2015 at 12:39:27PM +0100, LEROY Christophe wrote:
> > > The C version of csum_add() as defined in include/net/checksum.h gives the
> > > following assembly:
> > >        0:       7c 04 1a 14     add     r0,r4,r3
> > >        4:       7c 64 00 10     subfc   r3,r4,r0
> > >        8:       7c 63 19 10     subfe   r3,r3,r3
> > >        c:       7c 63 00 50     subf    r3,r3,r0
> > > 
> > > include/net/checksum.h also offers the possibility to define an arch specific
> > > function.
> > > This patch provides a ppc32 specific csum_add() inline function.
> > What makes it 32-bit specific?
> > 
> > 
> As far as I understand, the 64-bit will do a 64 bit addition, so we
> will have to handle differently the carry, can't just be an addze like
> in 32-bit.

OK.  Before I couldn't find where this was ifdeffed to 32-bit, but it's
in patch 1/2.

> The generated code is most likely different on ppc64. I have no ppc64
> compiler so I can't check what gcc generates for the following code:
> 
> __wsum csum_add(__wsum csum, __wsum addend)
> {
> 	u32 res = (__force u32)csum;
> 	res += (__force u32)addend;
> 	return (__force __wsum)(res + (res < (__force u32)addend));
> }
> 
> Can someone with a ppc64 compiler tell what we get ?

With CONFIG_GENERIC_CPU:

   0xc000000000001af8 <+0>:	add     r3,r3,r4
   0xc000000000001afc <+4>:	cmplw   cr7,r3,r4
   0xc000000000001b00 <+8>:	mfcr    r4
   0xc000000000001b04 <+12>:	rlwinm  r4,r4,29,31,31
   0xc000000000001b08 <+16>:	add     r3,r4,r3
   0xc000000000001b0c <+20>:	clrldi  r3,r3,32
   0xc000000000001b10 <+24>:	blr

The mfcr is particularly nasty, at least on our chips.

With CONFIG_CPU_E6500:

   0xc000000000001b30 <+0>:	add     r3,r3,r4
   0xc000000000001b34 <+4>:	cmplw   cr7,r3,r4
   0xc000000000001b38 <+8>:	mfocrf  r4,1
   0xc000000000001b3c <+12>:	rlwinm  r4,r4,29,31,31
   0xc000000000001b40 <+16>:	add     r3,r4,r3
   0xc000000000001b44 <+20>:	clrldi  r3,r3,32
   0xc000000000001b48 <+24>:	blr

Ideal (short of a 64-bit __wsum) would probably be something like (untested):

	add	r3,r3,r4
	srdi	r5,r3,32
	add	r3,r3,r5
	clrldi	r3,r3,32

Or in C code (which would let the compiler schedule it better):

static inline __wsum csum_add(__wsum csum, __wsum addend)
{
        u64 res = (__force u64)csum;
        res += (__force u32)addend;
        return (__force __wsum)((u32)res + (res >> 32));
}

-Scott

Re: [v2,2/2] powerpc32: add support for csum_add()

[Segher Boessenkool]

On Fri, May 01, 2015 at 08:00:14PM -0500, Scott Wood wrote:
> On Tue, 2015-04-28 at 21:01 +0200, christophe leroy wrote:
> > The generated code is most likely different on ppc64. I have no ppc64
> > compiler

For reference: yes you do.  Just add -m64.

> Ideal (short of a 64-bit __wsum) would probably be something like (untested):
> 
> 	add	r3,r3,r4
> 	srdi	r5,r3,32
> 	add	r3,r3,r5
> 	clrldi	r3,r3,32
> 
> Or in C code (which would let the compiler schedule it better):
> 
> static inline __wsum csum_add(__wsum csum, __wsum addend)
> {
>         u64 res = (__force u64)csum;
>         res += (__force u32)addend;
>         return (__force __wsum)((u32)res + (res >> 32));
> }

Older GCC make exactly your asm code for that, in 64-bit; newer GCC get
two adds (one as 32-bit, one as 64-bit, it does not see those are the
same, grrr); and GCC 5 makes the perfect  addc 3,4,3 ; addze 3,3  for
this in 32-bit mode.  You don't want to see what older GCC does with
32-bit though :-/


Segher

Re: [v2,2/2] powerpc32: add support for csum_add()

[leroy christophe]

Le 05/05/2015 00:10, Segher Boessenkool a écrit :
> On Fri, May 01, 2015 at 08:00:14PM -0500, Scott Wood wrote:
>> On Tue, 2015-04-28 at 21:01 +0200, christophe leroy wrote:
>>> The generated code is most likely different on ppc64. I have no ppc64
>>> compiler
> For reference: yes you do.  Just add -m64.
[root@localhost knl]# LANG= ppc-linux-gcc -m64 test.c
test.c:1:0: error: -m64 not supported in this configuration

Christophe