Re: [PATCH 04/12] RISC-V: memmove() size optimized version: Add comments

LGTM

On Wed, Apr 9, 2025 at 2:49 PM m fally <[email protected]> wrote:
>
> Since the algorithm in this version of memmove() is different
> from the original version, add comments to give a description.
>
> Reviewed-by: Christian Herber <[email protected]>
> Reviewed-by: Eric Salem <[email protected]>
> Signed-off-by: m fally <[email protected]>
> ---
>  newlib/libc/machine/riscv/memmove.S | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/newlib/libc/machine/riscv/memmove.S b/newlib/libc/machine/riscv/memmove.S
> index a77ac44be..36fe00eba 100644
> --- a/newlib/libc/machine/riscv/memmove.S
> +++ b/newlib/libc/machine/riscv/memmove.S
> @@ -14,21 +14,21 @@
>  .global memmove
>  .type  memmove, @function
>  memmove:
> -  beqz a2, .Ldone
> +  beqz a2, .Ldone              /* in case there are 0 bytes to be copied, return immediately */
>
> -  mv a4, a0
> +  mv a4, a0                    /* copy the destination address over to a4, since memmove should return that address in a0 at the end */
>    li a3, 1
> -  bgtu  a1, a0, .Lcopy
> +  bgtu  a1, a0, .Lcopy         /* in case of source address > destination address, copy from start to end of the specified memory area */
>
> -  li a3, -1
> -  add   a4, a4, a2
> -  add   a1, a1, a2
> +  li a3, -1                    /* otherwhise, we need to start copying from the end of the specified memory area, therefore after each copied byte, increment the addresses by -1 */
> +  add   a4, a4, a2             /* add the number of bytes to be copied to both addresses. this gives an incorrect address, */
> +  add   a1, a1, a2             /* therefore we need to subtract 1 from both addresses in the next step before starting the copying process. */
>
>  .Lincrement:
> -  add   a4, a4, a3
> +  add   a4, a4, a3             /* in case of source address < destination address, increment both addresses by -1 before copying any data to obtain the correct start addresses */
>    add   a1, a1, a3
>  .Lcopy:
> -  lb a5, 0(a1)
> +  lb a5, 0(a1)                 /* copy bytes as long as a2 (= the number of bytes to be copied) > 0 */
>    sb a5, 0(a4)
>    add   a2, a2, -1
>    bnez a2, .Lincrement
> --
> 2.49.0
>