* [INET] Optimise away a branch in IP_ECN_set_ce
@ 2004-09-09 0:03 Herbert Xu
2004-09-09 0:24 ` YOSHIFUJI Hideaki / 吉藤英明
0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2004-09-09 0:03 UTC (permalink / raw)
To: David S. Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 364 bytes --]
Hi Dave:
I was bored so here is a patch that optimises away a branch in
IP_ECN_set_ce.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1076 bytes --]
===== include/net/inet_ecn.h 1.7 vs edited =====
--- 1.7/include/net/inet_ecn.h 2004-09-09 08:04:37 +10:00
+++ edited/include/net/inet_ecn.h 2004-09-09 09:51:08 +10:00
@@ -49,19 +49,27 @@
static inline void IP_ECN_set_ce(struct iphdr *iph)
{
u32 check = iph->check;
+ u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
- switch (iph->tos & INET_ECN_MASK) {
- default:
- case INET_ECN_NOT_ECT:
- case INET_ECN_CE:
+ /*
+ * After the last operation we have (in binary):
+ * INET_ECN_NOT_ECT => 01
+ * INET_ECN_ECT_1 => 10
+ * INET_ECN_ECT_0 => 11
+ * INET_ECN_CE => 00
+ */
+ if (!(ecn & 2))
return;
- case INET_ECN_ECT_1:
- check += __constant_htons(0xFFFD);
- break;
- case INET_ECN_ECT_0:
- check += __constant_htons(0xFFFE);
- break;
- }
+
+ /*
+ * The following gives us:
+ * INET_ECN_ECT_1 => check += __constant_htons(0xFFFD)
+ * INET_ECN_ECT_0 => check += __constant_htons(0xFFFE)
+ */
+ if (__constant_htons(1) != 1)
+ ecn <<= 8;
+ check += __constant_htons(0xFFFB) + ecn;
+
iph->check = check + (check>=0xFFFF);
iph->tos |= INET_ECN_CE;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 0:03 [INET] Optimise away a branch in IP_ECN_set_ce Herbert Xu
@ 2004-09-09 0:24 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 0:27 ` Arnaldo Carvalho de Melo
2004-09-09 1:10 ` Herbert Xu
0 siblings, 2 replies; 10+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-09-09 0:24 UTC (permalink / raw)
To: herbert; +Cc: davem, netdev, yoshfuji
In article <20040909000330.GA5581@gondor.apana.org.au> (at Thu, 9 Sep 2004 10:03:31 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> + u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
:
> + /*
> + * The following gives us:
> + * INET_ECN_ECT_1 => check += __constant_htons(0xFFFD)
> + * INET_ECN_ECT_0 => check += __constant_htons(0xFFFE)
> + */
> + if (__constant_htons(1) != 1)
> + ecn <<= 8;
> + check += __constant_htons(0xFFFB) + ecn;
> +
> iph->check = check + (check>=0xFFFF);
> iph->tos |= INET_ECN_CE;
> }
s/__constant_htons/htons/g here, please.
And, I think
u16 ecn = (iph->tos + 1) & INET_ECN_MASK;
:
check += (u32)htons(0xfffb) + (u32)htons(ecn);
or something like that?
--
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 0:24 ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-09-09 0:27 ` Arnaldo Carvalho de Melo
2004-09-09 0:36 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 1:10 ` Herbert Xu
1 sibling, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2004-09-09 0:27 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / 吉藤英明
Cc: herbert, davem, netdev
Em Qua 08 Set 2004 21:24, YOSHIFUJI Hideaki / 吉藤英明 escreveu:
> In article <20040909000330.GA5581@gondor.apana.org.au> (at Thu, 9 Sep 2004
10:03:31 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> > + u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
> >
> > + /*
> > + * The following gives us:
> > + * INET_ECN_ECT_1 => check += __constant_htons(0xFFFD)
> > + * INET_ECN_ECT_0 => check += __constant_htons(0xFFFE)
> > + */
> > + if (__constant_htons(1) != 1)
> > + ecn <<= 8;
> > + check += __constant_htons(0xFFFB) + ecn;
> > +
> > iph->check = check + (check>=0xFFFF);
> > iph->tos |= INET_ECN_CE;
> > }
>
> s/__constant_htons/htons/g here, please.
For Herbert understanding: the generated code is the same in this case. :)
>
> And, I think
> u16 ecn = (iph->tos + 1) & INET_ECN_MASK;
>
> check += (u32)htons(0xfffb) + (u32)htons(ecn);
>
> or something like that?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 0:27 ` Arnaldo Carvalho de Melo
@ 2004-09-09 0:36 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 0:53 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-09-09 0:36 UTC (permalink / raw)
To: herbert, acme; +Cc: davem, netdev, yoshfuji
In article <200409082127.57827.acme@conectiva.com.br> (at Wed, 8 Sep 2004 21:27:57 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:
> Em Qua 08 Set 2004 21:24, YOSHIFUJI Hideaki / 吉藤英明 escreveu:
> > In article <20040909000330.GA5581@gondor.apana.org.au> (at Thu, 9 Sep 2004
> 10:03:31 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> > > + u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
> > >
> > > + /*
> > > + * The following gives us:
> > > + * INET_ECN_ECT_1 => check += __constant_htons(0xFFFD)
> > > + * INET_ECN_ECT_0 => check += __constant_htons(0xFFFE)
> > > + */
> > > + if (__constant_htons(1) != 1)
> > > + ecn <<= 8;
> > > + check += __constant_htons(0xFFFB) + ecn;
> > > +
> > > iph->check = check + (check>=0xFFFF);
> > > iph->tos |= INET_ECN_CE;
> > > }
> >
> > s/__constant_htons/htons/g here, please.
>
> For Herbert understanding: the generated code is the same in this case. :)
Yes, thanks.
And, we don't use __const_{hton,ntoh}{s,l}() unless it is really required.
e.g. variable initializer
--yoshfuji
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 0:36 ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-09-09 0:53 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2004-09-09 0:53 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / 吉藤英明
Cc: herbert, davem, netdev
Em Qua 08 Set 2004 21:36, YOSHIFUJI Hideaki / 吉藤英明 escreveu:
> In article <200409082127.57827.acme@conectiva.com.br> (at Wed, 8 Sep 2004
21:27:57 -0300), Arnaldo Carvalho de Melo <acme@conectiva.com.br> says:
> > Em Qua 08 Set 2004 21:24, YOSHIFUJI Hideaki / 吉藤英明 escreveu:
> > > In article <20040909000330.GA5581@gondor.apana.org.au> (at Thu, 9 Sep
> > > 2004
> >
> > 10:03:31 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> > > > + u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
> > > >
> > > > + /*
> > > > + * The following gives us:
> > > > + * INET_ECN_ECT_1 => check += __constant_htons(0xFFFD)
> > > > + * INET_ECN_ECT_0 => check += __constant_htons(0xFFFE)
> > > > + */
> > > > + if (__constant_htons(1) != 1)
> > > > + ecn <<= 8;
> > > > + check += __constant_htons(0xFFFB) + ecn;
> > > > +
> > > > iph->check = check + (check>=0xFFFF);
> > > > iph->tos |= INET_ECN_CE;
> > > > }
> > >
> > > s/__constant_htons/htons/g here, please.
> >
> > For Herbert understanding: the generated code is the same in this case.
> > :)
>
> Yes, thanks.
> And, we don't use __const_{hton,ntoh}{s,l}() unless it is really required.
> e.g. variable initializer
Yes, that is the only case, like I learned in my early janitor days, using the
__constant_ variations is not needed and makes the code needlessly ugly :)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 0:24 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 0:27 ` Arnaldo Carvalho de Melo
@ 2004-09-09 1:10 ` Herbert Xu
2004-09-09 1:27 ` YOSHIFUJI Hideaki / 吉藤英明
1 sibling, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2004-09-09 1:10 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / ?$B5HF#1QL@; +Cc: davem, netdev
[-- Attachment #1: Type: text/plain, Size: 667 bytes --]
On Thu, Sep 09, 2004 at 09:24:28AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>
> s/__constant_htons/htons/g here, please.
Good point.
> And, I think
> u16 ecn = (iph->tos + 1) & INET_ECN_MASK;
> check += (u32)htons(0xfffb) + (u32)htons(ecn);
The second htons is less optimal than the shift because htons doesn't
know that ecn is only a byte. So it'll end up doing a full swap.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1032 bytes --]
===== include/net/inet_ecn.h 1.7 vs edited =====
--- 1.7/include/net/inet_ecn.h 2004-09-09 08:04:37 +10:00
+++ edited/include/net/inet_ecn.h 2004-09-09 10:51:43 +10:00
@@ -49,19 +49,27 @@
static inline void IP_ECN_set_ce(struct iphdr *iph)
{
u32 check = iph->check;
+ u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
- switch (iph->tos & INET_ECN_MASK) {
- default:
- case INET_ECN_NOT_ECT:
- case INET_ECN_CE:
+ /*
+ * After the last operation we have (in binary):
+ * INET_ECN_NOT_ECT => 01
+ * INET_ECN_ECT_1 => 10
+ * INET_ECN_ECT_0 => 11
+ * INET_ECN_CE => 00
+ */
+ if (!(ecn & 2))
return;
- case INET_ECN_ECT_1:
- check += __constant_htons(0xFFFD);
- break;
- case INET_ECN_ECT_0:
- check += __constant_htons(0xFFFE);
- break;
- }
+
+ /*
+ * The following gives us:
+ * INET_ECN_ECT_1 => check += htons(0xFFFD)
+ * INET_ECN_ECT_0 => check += htons(0xFFFE)
+ */
+ if (htons(1) != 1)
+ ecn <<= 8;
+ check += htons(0xFFFB) + ecn;
+
iph->check = check + (check>=0xFFFF);
iph->tos |= INET_ECN_CE;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 1:10 ` Herbert Xu
@ 2004-09-09 1:27 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 1:54 ` Herbert Xu
0 siblings, 1 reply; 10+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-09-09 1:27 UTC (permalink / raw)
To: herbert; +Cc: davem, netdev, yoshfuji
In article <20040909011016.GA9238@gondor.apana.org.au> (at Thu, 9 Sep 2004 11:10:16 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> On Thu, Sep 09, 2004 at 09:24:28AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> >
> > s/__constant_htons/htons/g here, please.
>
> Good point.
>
> > And, I think
> > u16 ecn = (iph->tos + 1) & INET_ECN_MASK;
> > check += (u32)htons(0xfffb) + (u32)htons(ecn);
>
> The second htons is less optimal than the shift because htons doesn't
> know that ecn is only a byte. So it'll end up doing a full swap.
Not true; gcc is clever enough to produces same code, and my code looks better.
--
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 1:27 ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-09-09 1:54 ` Herbert Xu
2004-09-09 1:58 ` YOSHIFUJI Hideaki / 吉藤英明
0 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2004-09-09 1:54 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / ?$B5HF#1QL@; +Cc: davem, netdev
[-- Attachment #1: Type: text/plain, Size: 767 bytes --]
On Thu, Sep 09, 2004 at 10:27:36AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>
> > > And, I think
> > > u16 ecn = (iph->tos + 1) & INET_ECN_MASK;
> > > check += (u32)htons(0xfffb) + (u32)htons(ecn);
> >
> > The second htons is less optimal than the shift because htons doesn't
> > know that ecn is only a byte. So it'll end up doing a full swap.
>
> Not true; gcc is clever enough to produces same code, and my code looks better.
Sorry, you're right. Here is an updated patch.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1004 bytes --]
===== include/net/inet_ecn.h 1.7 vs edited =====
--- 1.7/include/net/inet_ecn.h 2004-09-09 08:04:37 +10:00
+++ edited/include/net/inet_ecn.h 2004-09-09 11:45:45 +10:00
@@ -49,19 +49,25 @@
static inline void IP_ECN_set_ce(struct iphdr *iph)
{
u32 check = iph->check;
+ u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
- switch (iph->tos & INET_ECN_MASK) {
- default:
- case INET_ECN_NOT_ECT:
- case INET_ECN_CE:
+ /*
+ * After the last operation we have (in binary):
+ * INET_ECN_NOT_ECT => 01
+ * INET_ECN_ECT_1 => 10
+ * INET_ECN_ECT_0 => 11
+ * INET_ECN_CE => 00
+ */
+ if (!(ecn & 2))
return;
- case INET_ECN_ECT_1:
- check += __constant_htons(0xFFFD);
- break;
- case INET_ECN_ECT_0:
- check += __constant_htons(0xFFFE);
- break;
- }
+
+ /*
+ * The following gives us:
+ * INET_ECN_ECT_1 => check += htons(0xFFFD)
+ * INET_ECN_ECT_0 => check += htons(0xFFFE)
+ */
+ check += htons(0xFFFB) + htons(ecn);
+
iph->check = check + (check>=0xFFFF);
iph->tos |= INET_ECN_CE;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [INET] Optimise away a branch in IP_ECN_set_ce
2004-09-09 1:54 ` Herbert Xu
@ 2004-09-09 1:58 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 4:35 ` David S. Miller
0 siblings, 1 reply; 10+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-09-09 1:58 UTC (permalink / raw)
To: davem, herbert; +Cc: netdev, yoshfuji
In article <20040909015442.GA12935@gondor.apana.org.au> (at Thu, 9 Sep 2004 11:54:42 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> Sorry, you're right. Here is an updated patch.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
--
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2004-09-09 4:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-09 0:03 [INET] Optimise away a branch in IP_ECN_set_ce Herbert Xu
2004-09-09 0:24 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 0:27 ` Arnaldo Carvalho de Melo
2004-09-09 0:36 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 0:53 ` Arnaldo Carvalho de Melo
2004-09-09 1:10 ` Herbert Xu
2004-09-09 1:27 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 1:54 ` Herbert Xu
2004-09-09 1:58 ` YOSHIFUJI Hideaki / 吉藤英明
2004-09-09 4:35 ` David S. Miller
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).