All of lore.kernel.org
 help / color / mirror / Atom feed
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?


  reply	other threads:[~2018-11-06 12:51 UTC|newest]

Thread overview: 32+ 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 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 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-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.