From: David Laight <David.Laight@ACULAB.COM>
To: 'Vlastimil Babka' <vbabka@suse.cz>,
'Bart Van Assche' <bvanassche@acm.org>,
Andrew Morton <akpm@linux-foundation.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Mel Gorman <mgorman@techsingularity.net>,
Christoph Lameter <cl@linux.com>, "Roman Gushchin" <guro@fb.com>
Subject: RE: [PATCH] slab.h: Avoid using & for logical and of booleans
Date: Tue, 6 Nov 2018 11:07:46 +0000 [thread overview]
Message-ID: <3c9adab0f1f74c46a60b3d4401030337@AcuMS.aculab.com> (raw)
In-Reply-To: <e44e6c8b-e4e4-e7cb-a5ca-88e9559eb0d7@suse.cz>
From: Vlastimil Babka [mailto:vbabka@suse.cz]
> Sent: 06 November 2018 10:22
...
> >> - return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> >> + return type_dma + is_reclaimable * !is_dma * KMALLOC_RECLAIM;
> >
> > ISTM that changing is_dma and is_reclaimable from int to bool will stop the bleating.
> >
> > It is also strange that this code is trying so hard here to avoid conditional instructions
I've done some experiments, compiled with gcc 4.7.3 and -O2
The constants match those from the kernel headers.
It is noticable that there isn't a cmov in sight.
The code would also be better if the KMALLOC constants matched the GFP ones.
#define __GFP_DMA 1u
#define __GFP_RECLAIM 0x10u
#define KMALLOC_DMA 2
#define KMALLOC_RECLAIM 1
unsigned int f(unsigned int flags)
{
return flags & __GFP_DMA ? KMALLOC_DMA : flags & __GFP_RECLAIM ? KMALLOC_RECLAIM : 0;
}
unsigned int f1(unsigned int flags)
{
return !__builtin_expect(flags & (__GFP_DMA | __GFP_RECLAIM), 0) ? 0 : flags & __GFP_DMA ? KMALLOC_DMA : KMALLOC_RECLAIM;
}
unsigned int f2(unsigned int flags)
{
int is_dma, type_dma, is_rec;
is_dma = !!(flags & __GFP_DMA);
type_dma = is_dma * KMALLOC_DMA;
is_rec = !!(flags & __GFP_RECLAIM);
return type_dma + (is_rec & !is_dma) * KMALLOC_RECLAIM;
}
unsigned int f3(unsigned int flags)
{
int is_dma, type_dma, is_rec;
is_dma = !!(flags & __GFP_DMA);
type_dma = is_dma * KMALLOC_DMA;
is_rec = !!(flags & __GFP_RECLAIM);
return type_dma + (is_rec * !is_dma) * KMALLOC_RECLAIM;
}
Disassembly of section .text:
0000000000000000 <f>:
0: 40 f6 c7 01 test $0x1,%dil
4: b8 02 00 00 00 mov $0x2,%eax
9: 74 05 je 10 <f+0x10>
b: f3 c3 repz retq
d: 0f 1f 00 nopl (%rax)
10: 89 f8 mov %edi,%eax
12: c1 e8 04 shr $0x4,%eax
15: 83 e0 01 and $0x1,%eax
18: c3 retq
This one has a misprediced branch in the common path.
0000000000000020 <f1>:
20: 40 f6 c7 11 test $0x11,%dil
24: 75 03 jne 29 <f1+0x9>
26: 31 c0 xor %eax,%eax
28: c3 retq
29: 83 e7 01 and $0x1,%edi
2c: 83 ff 01 cmp $0x1,%edi
2f: 19 c0 sbb %eax,%eax
31: 83 c0 02 add $0x2,%eax
34: c3 retq
The jne will be predicted not taken and the retq predicted.
So this might only be 1 clock in the normal case.
0000000000000040 <f2>:
40: 89 f8 mov %edi,%eax
42: c1 ef 04 shr $0x4,%edi
45: 83 e0 01 and $0x1,%eax
48: 89 c2 mov %eax,%edx
4a: 83 f2 01 xor $0x1,%edx
4d: 21 d7 and %edx,%edi
4f: 8d 04 47 lea (%rdi,%rax,2),%eax
52: c3 retq
No conditionals, but a 4 deep dependency chain.
0000000000000060 <f3>:
60: 89 fa mov %edi,%edx
62: c1 ef 04 shr $0x4,%edi
65: 83 e2 01 and $0x1,%edx
68: 83 e7 01 and $0x1,%edi
6b: 89 d0 mov %edx,%eax
6d: 83 f0 01 xor $0x1,%eax
70: 0f af c7 imul %edi,%eax
73: 8d 04 50 lea (%rax,%rdx,2),%eax
76: c3 retq
You really don't want the multiply.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2018-11-06 11:07 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-05 20:40 [PATCH] slab.h: Avoid using & for logical and of booleans Bart Van Assche
2018-11-05 21:13 ` Andrew Morton
2018-11-05 21:48 ` Bart Van Assche
2018-11-05 22:14 ` Rasmus Villemoes
2018-11-05 22:40 ` Bart Van Assche
2018-11-05 22:48 ` Alexander Duyck
2018-11-06 0:01 ` Bart Van Assche
2018-11-06 0:11 ` Alexander Duyck
2018-11-06 0:32 ` Bart Van Assche
2018-11-06 17:20 ` Alexander Duyck
2018-11-06 17:48 ` Bart Van Assche
2018-11-06 18:17 ` Alexander Duyck
2018-11-06 9:45 ` William Kucharski
2018-11-06 8:40 ` Vlastimil Babka
2018-11-06 10:08 ` David Laight
2018-11-06 10:22 ` Vlastimil Babka
2018-11-06 11:07 ` David Laight [this message]
2018-11-06 12:51 ` Vlastimil Babka
2018-11-07 10:41 ` David Laight
2018-11-09 8:12 ` Vlastimil Babka
2018-11-09 19:00 ` Andrew Morton
2018-11-09 19:16 ` Vlastimil Babka
2018-11-09 19:47 ` Darryl T. Agostinelli
2018-11-09 21:31 ` Vlastimil Babka
2018-11-12 9:55 ` David Laight
2018-11-13 18:22 ` Vlastimil Babka
2018-11-21 13:22 ` Vlastimil Babka
2018-11-19 11:04 ` Pavel Machek
2018-11-19 12:51 ` Vlastimil Babka
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=3c9adab0f1f74c46a60b3d4401030337@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=bvanassche@acm.org \
--cc=cl@linux.com \
--cc=guro@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@techsingularity.net \
--cc=vbabka@suse.cz \
/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).