mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sultan Alsawaf <sultan@kerneltoast.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	Nathan Chancellor <natechancellor@gmail.com>
Subject: Re: [RFCv2] string: Use faster alternatives when constant arguments are used
Date: Sat, 30 Mar 2019 15:59:41 -0700	[thread overview]
Message-ID: <20190330225941.GA7456@sultan-box.localdomain> (raw)
In-Reply-To: <8672b98c-bf71-7b5b-625e-2f241807d46c@rasmusvillemoes.dk>

On Mon, Mar 25, 2019 at 10:24:00PM +0100, Rasmus Villemoes wrote:
> What I'm worried about is your patch changing every single strcmp(,
> "literal") into a memcmp, with absolutely no way of knowing or checking
> anything about the other buffer. And actually, it doesn't have to be a
> BE arch with a word-at-a-time memcmp.
> If (as is usually the case) the strcmp() result is compared to zero, after you
> change
> 
>   !strcmp(buf, "literal")
> 
> into
> 
>   !memcmp(buf, "literal", 8)
> 
> the compiler may (exactly as you want it to) change that into a single
> 8-byte load (or two 4-byte loads) and comparisons to literals, no
> memcmp() involved. And how do you know that _that_ is ok, for every one
> of the hundreds, if not thousands, of instances in the tree?

When would this not be ok though? From what I've always known,

  strcmp(terminated_buf1, terminated_buf2)

is equivalent to

  memcmp(terminated_buf1, terminated_buf2, strlen(terminated_buf1))

  and

  memcmp(terminated_buf1, terminated_buf2, strlen(terminated_buf2))

regardless of whether or not one side is a literal (my patch just leverages the
compiler's ability to recognize strlen called on literals and optimize it out).
The latter memcmp instances would indeed perform worse than the first strcmp
when neither arguments are literals, but I don't see what makes the memcmp usage
"dangerous". How can the memcmps cross a page boundary when memcmp itself will
only read in large buffers of data at word boundaries?

And if there are concerns for some arches but not others, then couldn't this be
a feasible optimization for those which would work well with it?

Sultan

  reply	other threads:[~2019-03-30 22:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-24  1:44 [RFC] " Sultan Alsawaf
2019-03-24  2:24 ` [RFCv2] " Sultan Alsawaf
2019-03-24  3:31   ` Nathan Chancellor
2019-03-24  7:18     ` [RFC v3] " Sultan Alsawaf
2019-03-24 21:17   ` [RFCv2] " Rasmus Villemoes
2019-03-24 22:32     ` Sultan Alsawaf
2019-03-25 21:24       ` Rasmus Villemoes
2019-03-30 22:59         ` Sultan Alsawaf [this message]
2019-04-01 20:43           ` Rasmus Villemoes
2019-04-01 23:55             ` Sultan Alsawaf
2019-04-02  8:17               ` Rasmus Villemoes

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=20190330225941.GA7456@sultan-box.localdomain \
    --to=sultan@kerneltoast.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=natechancellor@gmail.com \
    /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

all inboxes | Powered by JetHome®