Received-SPF: pass (google.com: domain of linux-kernel+bounces-111308-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240314165715.31831-1-adobriyan@gmail.com>
In-Reply-To: <20240314165715.31831-1-adobriyan@gmail.com>
From: Uros Bizjak <ubizjak@gmail.com>
Date: Fri, 22 Mar 2024 11:37:31 +0100
Message-ID: <CAFULd4YrDiu7zUdZXRkLNiN=6zC4hPWchymrbMA=ag3qG+OarA@mail.gmail.com>
Subject: Re: [PATCH 1/1] x86_64: cleanup memset16(), memset32(), memset64()
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, 
	dave.hansen@linux.intel.com, x86@kernel.org, linux-kernel@vger.kernel.org, 
	hpa@zytor.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 22, 2024 at 11:05=E2=80=AFAM Alexey Dobriyan <adobriyan@gmail.c=
om> wrote:
>
> * use "+" constraint modifier,
>   simplify inputs and output lists,
>   delete dummy variables with meaningless names,
>
>   "&" only makes sense in complex assembly creating constraints on
>   intermediate registers. But 1 instruction assemblies don't have
>   inner body so to speak.
>
> * write "rep stos*" on one line:
>   Rep prefix is integral part of x86 instruction. I'm not sure why
>   people separate "rep" with newline or semicolon pretending rep is
>   separate instruction. It is not and never was.

The reason for this was that some archaic assemblers rejected "rep
insn" on one line. I have checked that the minimum required
binutils-2.25 assembles this without problems.

> * use __auto_type for maximum copy pasta experience,
>
> * reformat a bit to make everything looks nicer.
>
> Note that "memory" clobber is too much if "n" is known at compile time.
> However,
>
>         "=3Dm" (*(T(*)[n])s)
>
> doesn't work because -Wvla even if "n" is compile time constant:
>
>         if (BCP(n)) {
>                 rep stos
>                 : "=3Dm" (*(T(*)[n])s)
>         } else {
>                 rep stosw
>                 : "memory"
>         }
>
> The above doesn't work.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Reviewed-by: Uros Bizjak <ubizjak@gmail.com>

> ---
>  arch/x86/include/asm/string_64.h | 45 +++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/stri=
ng_64.h
> index 857d364b9888..9d0b324eab21 100644
> --- a/arch/x86/include/asm/string_64.h
> +++ b/arch/x86/include/asm/string_64.h
> @@ -30,37 +30,40 @@ void *__memset(void *s, int c, size_t n);
>  #define __HAVE_ARCH_MEMSET16
>  static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
>  {
> -       long d0, d1;
> -       asm volatile("rep\n\t"
> -                    "stosw"
> -                    : "=3D&c" (d0), "=3D&D" (d1)
> -                    : "a" (v), "1" (s), "0" (n)
> -                    : "memory");
> -       return s;
> +       const __auto_type s0 =3D s;
> +       asm volatile (
> +               "rep stosw"
> +               : "+D" (s), "+c" (n)
> +               : "a" (v)
> +               : "memory"
> +       );
> +       return s0;
>  }
>
>  #define __HAVE_ARCH_MEMSET32
>  static inline void *memset32(uint32_t *s, uint32_t v, size_t n)
>  {
> -       long d0, d1;
> -       asm volatile("rep\n\t"
> -                    "stosl"
> -                    : "=3D&c" (d0), "=3D&D" (d1)
> -                    : "a" (v), "1" (s), "0" (n)
> -                    : "memory");
> -       return s;
> +       const __auto_type s0 =3D s;
> +       asm volatile (
> +               "rep stosl"
> +               : "+D" (s), "+c" (n)
> +               : "a" (v)
> +               : "memory"
> +       );
> +       return s0;
>  }
>
>  #define __HAVE_ARCH_MEMSET64
>  static inline void *memset64(uint64_t *s, uint64_t v, size_t n)
>  {
> -       long d0, d1;
> -       asm volatile("rep\n\t"
> -                    "stosq"
> -                    : "=3D&c" (d0), "=3D&D" (d1)
> -                    : "a" (v), "1" (s), "0" (n)
> -                    : "memory");
> -       return s;
> +       const __auto_type s0 =3D s;
> +       asm volatile (
> +               "rep stosq"
> +               : "+D" (s), "+c" (n)
> +               : "a" (v)
> +               : "memory"
> +       );
> +       return s0;
>  }
>  #endif
>
> --
> 2.43.2
>