From: Vlastimil Babka <vbabka@suse.cz>
To: David Laight <David.Laight@ACULAB.COM>,
'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 13:51:12 +0100 [thread overview]
Message-ID: <60deb90d-e521-39e5-5072-fc9efb98e365@suse.cz> (raw)
In-Reply-To: <3c9adab0f1f74c46a60b3d4401030337@AcuMS.aculab.com>
On 11/6/18 12:07 PM, David Laight wrote:
> 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.
There is with newer gcc: https://godbolt.org/z/qFdByQ
But even that didn't remove the imul in f3() so that's indeed a bust.
> The code would also be better if the KMALLOC constants matched the GFP ones.
That would be hard, as __GFP flags have also other constraints
(especially __GFP_DMA relative to other zone restricting __GFP flags)
and KMALLOC_* are used as array index.
> unsigned int f1(unsigned int flags)
> {
> return !__builtin_expect(flags & (__GFP_DMA | __GFP_RECLAIM), 0) ? 0 : flags & __GFP_DMA ? KMALLOC_DMA : KMALLOC_RECLAIM;
> }
>
...
> 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.
I think this is the winner. It's also a single branch and not two,
because the compiler could figure out some of the "clever arithmetics"
itself. Care to send a full patch?
next prev parent reply other threads:[~2018-11-06 12:51 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
2018-11-06 12:51 ` Vlastimil Babka [this message]
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=60deb90d-e521-39e5-5072-fc9efb98e365@suse.cz \
--to=vbabka@suse.cz \
--cc=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 \
/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).