From: David Laight <David.Laight@ACULAB.COM>
To: 'Christophe Leroy' <christophe.leroy@csgroup.eu>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] net: Remove branch in csum_shift()
Date: Sun, 13 Feb 2022 02:39:06 +0000 [thread overview]
Message-ID: <7f16910a8f63475dae012ef5135f41d1@AcuMS.aculab.com> (raw)
In-Reply-To: <efeeb0b9979b0377cd313311ad29cf0ac060ae4b.1644569106.git.christophe.leroy@csgroup.eu>
From: Christophe Leroy
> Sent: 11 February 2022 08:48
>
> Today's implementation of csum_shift() leads to branching based on
> parity of 'offset'
>
> 000002f8 <csum_block_add>:
> 2f8: 70 a5 00 01 andi. r5,r5,1
> 2fc: 41 a2 00 08 beq 304 <csum_block_add+0xc>
> 300: 54 84 c0 3e rotlwi r4,r4,24
> 304: 7c 63 20 14 addc r3,r3,r4
> 308: 7c 63 01 94 addze r3,r3
> 30c: 4e 80 00 20 blr
>
> Use first bit of 'offset' directly as input of the rotation instead of
> branching.
>
> 000002f8 <csum_block_add>:
> 2f8: 54 a5 1f 38 rlwinm r5,r5,3,28,28
> 2fc: 20 a5 00 20 subfic r5,r5,32
> 300: 5c 84 28 3e rotlw r4,r4,r5
> 304: 7c 63 20 14 addc r3,r3,r4
> 308: 7c 63 01 94 addze r3,r3
> 30c: 4e 80 00 20 blr
>
> And change to left shift instead of right shift to skip one more
> instruction. This has no impact on the final sum.
>
> 000002f8 <csum_block_add>:
> 2f8: 54 a5 1f 38 rlwinm r5,r5,3,28,28
> 2fc: 5c 84 28 3e rotlw r4,r4,r5
> 300: 7c 63 20 14 addc r3,r3,r4
> 304: 7c 63 01 94 addze r3,r3
> 308: 4e 80 00 20 blr
That is ppc64.
What happens on x86-64?
Trying to do the same in the x86 ipcsum code tended to make the code worse.
(Although that test is for an odd length fragment and can just be removed.)
David
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> include/net/checksum.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/net/checksum.h b/include/net/checksum.h
> index 5218041e5c8f..9badcd5532ef 100644
> --- a/include/net/checksum.h
> +++ b/include/net/checksum.h
> @@ -83,9 +83,7 @@ static inline __sum16 csum16_sub(__sum16 csum, __be16 addend)
> static inline __wsum csum_shift(__wsum sum, int offset)
> {
> /* rotate sum to align it with a 16b boundary */
> - if (offset & 1)
> - return (__force __wsum)ror32((__force u32)sum, 8);
> - return sum;
> + return (__force __wsum)rol32((__force u32)sum, (offset & 1) << 3);
> }
>
> static inline __wsum
> --
> 2.34.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2022-02-13 2:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-11 8:48 [PATCH] net: Remove branch in csum_shift() Christophe Leroy
2022-02-13 2:39 ` David Laight [this message]
2022-02-13 9:16 ` Segher Boessenkool
2022-02-13 17:47 ` David Laight
2022-02-14 9:29 ` Segher Boessenkool
2022-03-01 10:20 ` Christophe Leroy
2022-03-01 10:47 ` David Laight
2022-03-01 11:14 ` Christophe Leroy
2022-03-01 11:41 ` David Laight
2022-03-01 12:37 ` Russell King (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f16910a8f63475dae012ef5135f41d1@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=christophe.leroy@csgroup.eu \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).