linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Borislav Petkov <bp@alien8.de>,
	Rasmus Villemoes <mail@rasmusvillemoes.dk>,
	x86-ml <x86@kernel.org>, Andy Lutomirski <luto@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] Improve memset
Date: Mon, 16 Sep 2019 10:40:53 -0700	[thread overview]
Message-ID: <CALCETrX8sR8ELEvUpdHug498dU6+MWSy_SagaRbuZZ9fkztmfw@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wjdpJ+VapXfoZE8JRUfvMb8JrVTZe0=TDFYZ-ke+uqBOA@mail.gmail.com>

On Mon, Sep 16, 2019 at 10:25 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Sep 16, 2019 at 2:18 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
> >
> > Eh, this benchmark doesn't seem to provide any hints on where to set the
> > cut-off for a compile-time constant n, i.e. the 32 in
>
> Yes, you'd need to use proper fixed-size memset's with
> __builtin_memset() to test that case. Probably easy enough with some
> preprocessor macros to expand to a lot of cases.
>
> But even then it will not show some of the advantages of inlining the
> memset (quite often you have a "memset structure to zero, then
> initialize a couple of fields" pattern, and gcc does much better for
> that when it just inlines the memset to stores - to the point of just
> removing all the memset entirely and just storing a couple of zeroes
> between the fields you initialized).

After some experimentation, I think y'all are just doing it wrong.
GCC is very clever about this as long as it's given the chance.  This
test, for example, generates excellent code:

#include <string.h>

__THROW __nonnull ((1)) __attribute__((always_inline)) void
*memset(void *s, int c, size_t n)
{
    asm volatile ("nop");
    return s;
}

/* generates 'nop' */
void zero(void *dest, size_t size)
{
    __builtin_memset(dest, 0, size);
}

/* xorl %eax, %eax */
int test(void)
{
    int x;
    __builtin_memset(&x, 0, sizeof(x));
    return x;
}

/* movl $0, (%rdi) */
void memset_a_bit(int *ptr)
{
    __builtin_memset(ptr, 0, sizeof(*ptr));
}

So I'm thinking maybe compiler.h should actually do something like:

#define memset __builtin_memset

and we should have some appropriate magic so that the memset inline is
exempt from the macro.  Or maybe there's some very clever way to put
all of this into the memset inline function.  FWIW, this obviously
wrong code:

__THROW __nonnull ((1)) __attribute__((always_inline)) void
*memset(void *s, int c, size_t n)
{
    __builtin_memset(s, c, n);
    return s;
}

generates 'jmp memset'.  It's not entirely clear to me exactly what's
happening here.

--Andy

  reply	other threads:[~2019-09-16 17:41 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-13  7:22 [RFC] Improve memset Borislav Petkov
2019-09-13  7:35 ` Ingo Molnar
2019-09-13  7:50   ` Borislav Petkov
2019-09-13  8:51 ` Rasmus Villemoes
2019-09-13  9:00 ` Linus Torvalds
2019-09-13  9:18   ` Rasmus Villemoes
2019-09-13 10:42     ` Borislav Petkov
2019-09-13 16:36       ` Borislav Petkov
2019-09-16  9:18         ` Rasmus Villemoes
2019-09-16 17:25           ` Linus Torvalds
2019-09-16 17:40             ` Andy Lutomirski [this message]
2019-09-16 21:29               ` Linus Torvalds
2019-09-16 23:13                 ` Andy Lutomirski
2019-09-16 23:26                   ` Linus Torvalds
2019-09-17  8:15             ` Borislav Petkov
2019-09-17 10:55             ` David Laight
2019-09-17 20:10 ` Josh Poimboeuf
2019-09-17 20:45   ` Linus Torvalds
2019-09-19 12:55     ` Borislav Petkov
2019-09-19 12:49   ` Borislav Petkov
2019-09-14  9:29 Alexey Dobriyan
2019-09-14 11:39 ` Borislav Petkov

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=CALCETrX8sR8ELEvUpdHug498dU6+MWSy_SagaRbuZZ9fkztmfw@mail.gmail.com \
    --to=luto@kernel.org \
    --cc=bp@alien8.de \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mail@rasmusvillemoes.dk \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /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).