Messages in this thread | | | Date | Sat, 14 Sep 2019 18:15:37 +0300 | From | Alexey Dobriyan <> | Subject | Re: [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.
| |