Re: [PATCH] Replace __bzero with memset

[Zack Weinberg <zackw at panix dot com>]

On Mon, Jun 12, 2017 at 6:45 AM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Replace all internal uses of __bzero with memset.  This removes the need
> to redirect it to a builtin and means memset is inlined whenever possible,
> including with -Os.

+1 from me.

>         * string/bits/string2.h (__bzero): Remove define.

You've already got a patch outstanding (and approved, I think) that
removes this header entirely, and people seem to be having problems
with this piece, so I suggest you scrap this bit for now ...

>         * sunrpc/bindrsvprt.c (bindresvport): Change __bzero to memset.
>         * sunrpc/clnt_gen.c (clnt_create): Likewise.
>         * sunrpc/des_impl.c (_des_crypt): Likewise.
>         * sunrpc/key_call.c (key_gendes): Likewise.
>         * sunrpc/pmap_rmt.c (clnt_broadcast): Likewise.
>         * sunrpc/svc_simple.c (universal): Likewise.
>         * sunrpc/svc_tcp.c (svctcp_create): Likewise.
>         * sunrpc/svc_udp.c (svcudp_bufcreate): Likewise.
>         * sysdeps/arm/aeabi_memclr.c (__aeabi_memclr): Likewise.

... and just go ahead and commit these, then rev the removal of
bits/string2.h and maybe land that too?

> +/* Set memory like memset, but different argument order and no return
> +   value required.  Also only integer caller-saves may be used.  */
>  void
>  __aeabi_memclr (void *dest, size_t n)
>  {
> -  __bzero (dest, n);
> +  memset (dest, 0, n);
>  }

It's a pre-existing condition, so it shouldn't hold up your patch, but
this comment concerns me; there is no guarantee that memset will avoid
using FP or vector registers.  The existing arm/memset.S *doesn't*,
but it doesn't look like it's all that fine-tuned (compare
arm/armv7/multiarch/memcpy_impl.S).

zw