Status about csum_partial optimization patches.

Status about csum_partial optimization patches.

[cee1]

Hi all,

The csum_partial optimization patches have resided at patch-work for
quite a while:
1. http://patchwork.linux-mips.org/patch/6988/
2. http://patchwork.linux-mips.org/patch/7176/

Any comments about patch 1, can it be merged?

For patch 2, which is actually Loongson3 related and is modified from
ralf's patch: http://www.linux-mips.org/archives/linux-mips/2014-06/msg00023.html.
In patch 2, there are still two switches for Loongson 3A:
1. #define cpu_has_wsbh IS_ENABLED(CONFIG_CPU_LOONGSON3) in
cpu-feature-overrides.h
2. #if defined(_MIPS_ARCH_LOONGSON3A) in swab.h

It replaces the "#if defined(_MIPS_ARCH_LOONGSON3A)" with "#if
cpu_has_wsbh" in csum_partial.S, compared with ralf's original patch.
My question is using cpu_has_wsbh in macro is not suitable, hence it
is preferred to revert to the ralf's original patch?



-- 
Regards,

- cee1

[v2] mips: use wsbh/dsbh/dshd on Loongson 3A

[chenj]

Signed-off-by: chenj <chenj@lemote.com>
---
This patch is modified from http://patchwork.linux-mips.org/patch/7054/
The original author is ralf.

v2: using "#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_LOONGSON3)"
instead of "#if cpu_has_wsbh" in csum_partial.S

 arch/mips/include/asm/cpu-features.h                       | 10 ++++++++++
 .../include/asm/mach-cavium-octeon/cpu-feature-overrides.h |  1 +
 .../mips/include/asm/mach-loongson/cpu-feature-overrides.h |  2 ++
 arch/mips/include/uapi/asm/swab.h                          | 14 ++++++++++++--
 arch/mips/lib/csum_partial.S                               | 10 ++++++++--
 arch/mips/net/bpf_jit.c                                    |  2 +-
 6 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index e079598..3325f3e 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -231,6 +231,16 @@
 #define cpu_has_clo_clz	cpu_has_mips_r
 #endif
 
+/*
+ * MIPS32 R2, MIPS64 R2, Loongson 3A and Octeon have WSBH.
+ * MIPS64 R2, Loongson 3A and Octeon have WSBH, DSBH and DSHD.
+ * This indicates the availability of WSBH and in case of 64 bit CPUs also
+ * DSBH and DSHD.
+ */
+#ifndef cpu_has_wsbh
+#define cpu_has_wsbh		cpu_has_mips_r2
+#endif
+
 #ifndef cpu_has_dsp
 #define cpu_has_dsp		(cpu_data[0].ases & MIPS_ASE_DSP)
 #endif
diff --git a/arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h b/arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
index cf80228..fa1f3cf 100644
--- a/arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
+++ b/arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
@@ -57,6 +57,7 @@
 #define cpu_has_vint		0
 #define cpu_has_veic		0
 #define cpu_hwrena_impl_bits	0xc0000000
+#define cpu_has_wsbh            1
 
 #define cpu_has_rixi		(cpu_data[0].cputype != CPU_CAVIUM_OCTEON)
 
diff --git a/arch/mips/include/asm/mach-loongson/cpu-feature-overrides.h b/arch/mips/include/asm/mach-loongson/cpu-feature-overrides.h
index c0f3ef4..7d28f95 100644
--- a/arch/mips/include/asm/mach-loongson/cpu-feature-overrides.h
+++ b/arch/mips/include/asm/mach-loongson/cpu-feature-overrides.h
@@ -59,4 +59,6 @@
 #define cpu_has_watch		1
 #define cpu_has_local_ebase	0
 
+#define cpu_has_wsbh		IS_ENABLED(CONFIG_CPU_LOONGSON3)
+
 #endif /* __ASM_MACH_LOONGSON_CPU_FEATURE_OVERRIDES_H */
diff --git a/arch/mips/include/uapi/asm/swab.h b/arch/mips/include/uapi/asm/swab.h
index ac9a8f9..20b884a 100644
--- a/arch/mips/include/uapi/asm/swab.h
+++ b/arch/mips/include/uapi/asm/swab.h
@@ -13,12 +13,16 @@
 
 #define __SWAB_64_THRU_32__
 
-#if defined(__mips_isa_rev) && (__mips_isa_rev >= 2)
+#if (defined(__mips_isa_rev) && (__mips_isa_rev >= 2)) ||		\
+    defined(_MIPS_ARCH_LOONGSON3A)
 
 static inline __attribute_const__ __u16 __arch_swab16(__u16 x)
 {
 	__asm__(
+	"	.set	push			\n"
+	"	.set	arch=mips32r2		\n"
 	"	wsbh	%0, %1			\n"
+	"	.set	pop			\n"
 	: "=r" (x)
 	: "r" (x));
 
@@ -29,8 +33,11 @@ static inline __attribute_const__ __u16 __arch_swab16(__u16 x)
 static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
 {
 	__asm__(
+	"	.set	push			\n"
+	"	.set	arch=mips32r2		\n"
 	"	wsbh	%0, %1			\n"
 	"	rotr	%0, %0, 16		\n"
+	"	.set	pop			\n"
 	: "=r" (x)
 	: "r" (x));
 
@@ -46,8 +53,11 @@ static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
 static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
 {
 	__asm__(
+	"	.set	push			\n"
+	"	.set	arch=mips64r2		\n"
 	"	dsbh	%0, %1\n"
 	"	dshd	%0, %0"
+	"	.set	pop			\n"
 	: "=r" (x)
 	: "r" (x));
 
@@ -55,5 +65,5 @@ static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
 }
 #define __arch_swab64 __arch_swab64
 #endif /* __mips64 */
-#endif /* MIPS R2 or newer  */
+#endif /* MIPS R2 or newer or Loongson 3A */
 #endif /* _ASM_SWAB_H */
diff --git a/arch/mips/lib/csum_partial.S b/arch/mips/lib/csum_partial.S
index 9901237..4c721e2 100644
--- a/arch/mips/lib/csum_partial.S
+++ b/arch/mips/lib/csum_partial.S
@@ -277,9 +277,12 @@ LEAF(csum_partial)
 #endif
 
 	/* odd buffer alignment? */
-#ifdef CONFIG_CPU_MIPSR2
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_LOONGSON3)
+	.set	push
+	.set	arch=mips32r2
 	wsbh	v1, sum
 	movn	sum, v1, t7
+	.set	pop
 #else
 	beqz	t7, 1f			/* odd buffer alignment? */
 	 lui	v1, 0x00ff
@@ -726,9 +729,12 @@ LEAF(csum_partial)
 	addu	sum, v1
 #endif
 
-#ifdef CONFIG_CPU_MIPSR2
+#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_LOONGSON3)
+	.set	push
+	.set	arch=mips32r2
 	wsbh	v1, sum
 	movn	sum, v1, odd
+	.set	pop
 #else
 	beqz	odd, 1f			/* odd buffer alignment? */
 	 lui	v1, 0x00ff
diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index 05a5661..762f448 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -1262,7 +1262,7 @@ jmp_cmp:
 			emit_half_load(r_A, r_skb, off, ctx);
 #ifdef CONFIG_CPU_LITTLE_ENDIAN
 			/* This needs little endian fixup */
-			if (cpu_has_mips_r2) {
+			if (cpu_has_wsbh) {
 				/* R2 and later have the wsbh instruction */
 				emit_wsbh(r_A, r_A, ctx);
 			} else {
-- 
1.9.0

Re: [v2] mips: use wsbh/dsbh/dshd on Loongson 3A

[David Daney]

On 08/15/2014 01:56 AM, chenj wrote:
> Signed-off-by: chenj <chenj@lemote.com>
> ---
> This patch is modified from http://patchwork.linux-mips.org/patch/7054/
> The original author is ralf.
>
> v2: using "#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_LOONGSON3)"
> instead of "#if cpu_has_wsbh" in csum_partial.S
>
>   arch/mips/include/asm/cpu-features.h                       | 10 ++++++++++
>   .../include/asm/mach-cavium-octeon/cpu-feature-overrides.h |  1 +
>   .../mips/include/asm/mach-loongson/cpu-feature-overrides.h |  2 ++
>   arch/mips/include/uapi/asm/swab.h                          | 14 ++++++++++++--
>   arch/mips/lib/csum_partial.S                               | 10 ++++++++--
>   arch/mips/net/bpf_jit.c                                    |  2 +-
>   6 files changed, 34 insertions(+), 5 deletions(-)
>
[...]
> diff --git a/arch/mips/include/uapi/asm/swab.h b/arch/mips/include/uapi/asm/swab.h
> index ac9a8f9..20b884a 100644
> --- a/arch/mips/include/uapi/asm/swab.h
> +++ b/arch/mips/include/uapi/asm/swab.h
[...]
> @@ -46,8 +53,11 @@ static inline __attribute_const__ __u32 __arch_swab32(__u32 x)
>   static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
>   {
>   	__asm__(
> +	"	.set	push			\n"
> +	"	.set	arch=mips64r2		\n"
>   	"	dsbh	%0, %1\n"
>   	"	dshd	%0, %0"
> +	"	.set	pop			\n"
>   	: "=r" (x)
>   	: "r" (x));
>

This section of the patch is defective.  It appears to have not been 
compile tested.

On mips-for-linux-next commit d4c5edf76f14720a32805202129dfa8206560035 
produces:

.
.
.
   mips64-octeon-linux-gnu-gcc -Wp,-MD,kernel/bpf/.core.o.d  -nostdinc 
-isystem 
/nfs/sdk/daily/tools-140726/bin/../lib/gcc/mips64-octeon-linux-gnu/4.7.0/include 
-I./arch/mips/include -Iarch/mips/include/generated  -Iinclude 
-I./arch/mips/include/uapi -Iarch/mips/include/generated/uapi 
-I./include/uapi -Iinclude/generated/uapi -include 
./include/linux/kconfig.h -D__KERNEL__ 
-DVMLINUX_LOAD_ADDRESS=0xffffffff81100000 -DDATAOFFSET=0 -Wall -Wundef 
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common 
-Werror-implicit-function-declaration -Wno-format-security 
-mno-check-zero-division -mabi=64 -G 0 -mno-abicalls -fno-pic -pipe 
-msoft-float -ffreestanding -march=octeon -Wa,--trap -Wa,-mfix-cn63xxp1 
-I./arch/mips/include/asm/mach-cavium-octeon 
-I./arch/mips/include/asm/mach-generic -msym32 -DKBUILD_64BIT_SYM32 
-fno-delete-null-pointer-checks -O2 --param=allow-store-data-races=0 
-Wframe-larger-than=2048 -fno-stack-protector 
-Wno-unused-but-set-variable -fomit-frame-pointer 
-fno-var-tracking-assignments -g -Wdeclaration-after-statement 
-Wno-pointer-sign -fno-strict-overflow -fconserve-stack 
-Werror=implicit-int -Werror=strict-prototypes -DCC_HAVE_ASM_GOTO 
-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(core)" 
-D"KBUILD_MODNAME=KBUILD_STR(core)" -c -o kernel/bpf/core.o 
kernel/bpf/core.c
{standard input}: Assembler messages:
{standard input}:3016: Error: Illegal operands `dshd $3,$3 .set pop'
.
.
.


Each line except the final line needs to be terminated with a '\n', but 
you did not do this following the "dshd" line.

David Daney

Re: [v2] mips: use wsbh/dsbh/dshd on Loongson 3A

[Chen Jie]

2014-08-20 7:21 GMT+08:00 David Daney <ddaney.cavm@gmail.com>:
> On 08/15/2014 01:56 AM, chenj wrote:
>>
>> Signed-off-by: chenj <chenj@lemote.com>
>> ---
>> This patch is modified from http://patchwork.linux-mips.org/patch/7054/
>> The original author is ralf.
>>
>> v2: using "#if defined(CONFIG_CPU_MIPSR2) ||
>> defined(CONFIG_CPU_LOONGSON3)"
>> instead of "#if cpu_has_wsbh" in csum_partial.S
>>
>>   arch/mips/include/asm/cpu-features.h                       | 10
>> ++++++++++
>>   .../include/asm/mach-cavium-octeon/cpu-feature-overrides.h |  1 +
>>   .../mips/include/asm/mach-loongson/cpu-feature-overrides.h |  2 ++
>>   arch/mips/include/uapi/asm/swab.h                          | 14
>> ++++++++++++--
>>   arch/mips/lib/csum_partial.S                               | 10
>> ++++++++--
>>   arch/mips/net/bpf_jit.c                                    |  2 +-
>>   6 files changed, 34 insertions(+), 5 deletions(-)
>>
> [...]
>
>> diff --git a/arch/mips/include/uapi/asm/swab.h
>> b/arch/mips/include/uapi/asm/swab.h
>> index ac9a8f9..20b884a 100644
>> --- a/arch/mips/include/uapi/asm/swab.h
>> +++ b/arch/mips/include/uapi/asm/swab.h
>
> [...]
>
>> @@ -46,8 +53,11 @@ static inline __attribute_const__ __u32
>> __arch_swab32(__u32 x)
>>   static inline __attribute_const__ __u64 __arch_swab64(__u64 x)
>>   {
>>         __asm__(
>> +       "       .set    push                    \n"
>> +       "       .set    arch=mips64r2           \n"
>>         "       dsbh    %0, %1\n"
>>         "       dshd    %0, %0"
>> +       "       .set    pop                     \n"
>>         : "=r" (x)
>>         : "r" (x));
>>
>
> This section of the patch is defective.  It appears to have not been compile
> tested.
>
> On mips-for-linux-next commit d4c5edf76f14720a32805202129dfa8206560035
> produces:
Really sorry for that, I've submitted a patch for this:
http://patchwork.linux-mips.org/patch/7550/

Re: Status about csum_partial optimization patches.

[cee1]

2014-07-31 10:55 GMT+08:00 cee1 <fykcee1@gmail.com>:
> Hi all,
>
> The csum_partial optimization patches have resided at patch-work for
> quite a while:
> 1. http://patchwork.linux-mips.org/patch/6988/
> 2. http://patchwork.linux-mips.org/patch/7176/
Patch 2 has already been merged, "commit
e309a3850f2e0fd9decd1be4ee778fa0b995202e -- MIPS: Use WSBH/DSBH/DSHD
on Loongson 3A"


>
> Any comments about patch 1, can it be merged?
In patch 1, the typical adjustment is as following:

""" /* original */
ADDC(sum, t0)
ADDC(sum, t1)
"""

Is replaced with
"""
ADDC(t0, t1)
ADDC(sum, t0)
"""

Hence, it needs to prove "sum ADDC t0 ADDC t1" is equal to "sum ADDC
(t0 ADDC t1)", I've added the explanation in
http://www.linux-mips.org/archives/linux-mips/2014-08/msg00124.html.



-- 
Regards,

- cee1