lkml.org 
[lkml]   [2019]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86_64: new and improved memset()
On Sat, Sep 14, 2019 at 01:37:17PM +0200, Borislav Petkov wrote:
> On Sat, Sep 14, 2019 at 01:33:45PM +0300, Alexey Dobriyan wrote:
> > --- a/arch/x86/include/asm/string_64.h
> > +++ b/arch/x86/include/asm/string_64.h
> > @@ -15,7 +15,111 @@ extern void *memcpy(void *to, const void *from, size_t len);
> > extern void *__memcpy(void *to, const void *from, size_t len);
> >
> > #define __HAVE_ARCH_MEMSET
> > +#if defined(_ARCH_X86_BOOT) || defined(CONFIG_FORTIFY_SOURCE)
> > void *memset(void *s, int c, size_t n);
> > +#else
> > +#include <asm/alternative.h>
> > +#include <asm/cpufeatures.h>
> > +
> > +/* Internal, do not use. */
> > +static __always_inline void memset0(void *s, size_t n)
> > +{
> > + /* Internal, do not use. */
> > + void _memset0_mov(void);
> > + void _memset0_rep_stosq(void);
> > + void memset0_mov(void);
> > + void memset0_rep_stosq(void);
> > + void memset0_rep_stosb(void);
> > +
> > + if (__builtin_constant_p(n) && n == 0) {
> > + } else if (__builtin_constant_p(n) && n == 1) {
> > + *(uint8_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && n == 2) {
> > + *(uint16_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && n == 4) {
> > + *(uint32_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && n == 6) {
> > + *(uint32_t *)s = 0;
> > + *(uint16_t *)(s + 4) = 0;
> > + } else if (__builtin_constant_p(n) && n == 8) {
> > + *(uint64_t *)s = 0;
> > + } else if (__builtin_constant_p(n) && (n & 7) == 0) {
> > + alternative_call_2(
> > + _memset0_mov,
> > + _memset0_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memset0_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n)
> > + : "rax", "cc", "memory"
> > + );
> > + } else {
> > + alternative_call_2(
> > + memset0_mov,
> > + memset0_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memset0_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n)
> > + : "rax", "rsi", "cc", "memory"
> > + );
> > + }
> > +}
> > +
> > +/* Internal, do not use. */
> > +static __always_inline void memsetx(void *s, int c, size_t n)
> > +{
> > + /* Internal, do not use. */
> > + void _memsetx_mov(void);
> > + void _memsetx_rep_stosq(void);
> > + void memsetx_mov(void);
> > + void memsetx_rep_stosq(void);
> > + void memsetx_rep_stosb(void);
> > +
> > + const uint64_t ccc = (uint8_t)c * 0x0101010101010101ULL;
> > +
> > + if (__builtin_constant_p(n) && n == 0) {
> > + } else if (__builtin_constant_p(n) && n == 1) {
> > + *(uint8_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && n == 2) {
> > + *(uint16_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && n == 4) {
> > + *(uint32_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && n == 8) {
> > + *(uint64_t *)s = ccc;
> > + } else if (__builtin_constant_p(n) && (n & 7) == 0) {
> > + alternative_call_2(
> > + _memsetx_mov,
> > + _memsetx_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memsetx_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n), "a" (ccc)
> > + : "cc", "memory"
> > + );
> > + } else {
> > + alternative_call_2(
> > + memsetx_mov,
> > + memsetx_rep_stosq, X86_FEATURE_REP_GOOD,
> > + memsetx_rep_stosb, X86_FEATURE_ERMS,
> > + ASM_OUTPUT2("=D" (s), "=c" (n)),
> > + "D" (s), "c" (n), "a" (ccc)
> > + : "rsi", "cc", "memory"
> > + );
> > + }
> > +}
> > +
> > +static __always_inline void *memset(void *s, int c, size_t n)
> > +{
> > + if (__builtin_constant_p(c)) {
> > + if (c == 0) {
> > + memset0(s, n);
> > + } else {
> > + memsetx(s, c, n);
> > + }
> > + return s;
> > + } else {
> > + return __builtin_memset(s, c, n);
> > + }
> > +}
>
> I'm willing to take something like that only when such complexity is
> justified by numbers. I.e., I'm much more inclined to capping it under
> 32 and 64 byte sizes and keeping it simple.

OK. Those small lengths were indeed annoying.

> > +ENTRY(_memset0_mov)
> > + xor eax, eax
> > +.globl _memsetx_mov
> > +_memsetx_mov:
> > + add rcx, rdi
> > + cmp rdi, rcx
> > + je 1f
> > +2:
> > + mov [rdi], rax
> > + add rdi, 8
> > + cmp rdi, rcx
> > + jne 2b
> > +1:
> > + ret
> > +ENDPROC(_memset0_mov)
> > +ENDPROC(_memsetx_mov)
> > +EXPORT_SYMBOL(_memset0_mov)
> > +EXPORT_SYMBOL(_memsetx_mov)
> > +
> > +ENTRY(memset0_mov)
> > + xor eax, eax
> > +.globl memsetx_mov
> > +memsetx_mov:
> > + lea rsi, [rdi + rcx]
> > + cmp rdi, rsi
> > + je 1f
> > +2:
> > + mov [rdi], al
> > + add rdi, 1
> > + cmp rdi, rsi
> > + jne 2b
> > +1:
> > + ret
>
> Say what now? Intel syntax? You must be joking...

It is the best thing in the x86 assembler universe.

> > +ENDPROC(memset0_mov)
> > +ENDPROC(memsetx_mov)
> > +EXPORT_SYMBOL(memset0_mov)
> > +EXPORT_SYMBOL(memsetx_mov)
>
> Too many exported symbols.

Those are technical exports. memset() remains the only developer-visible
interface.

> Again, I'll much more prefer a cleaner,
> smaller solution than one where readability suffers greatly at the
> expense of *maybe* getting a bit better performance.

Readability is red herring, I for one find AT&T syntax unreadable.

\
 
 \ /
  Last update: 2019-09-14 17:16    [W:0.069 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site