[PATCH][AArch64] Merge Falkor memcpy and memmove

Wilco Dijkstra Wilco.Dijkstra@arm.com
Mon Jun 8 13:25:56 GMT 2020


Hi Adhemerval,

Thanks for the review.

> -sysdep_routines += memcpy_generic memcpy_thunderx memcpy_thunderx2 \
> -                memcpy_falkor memmove_falkor \

> Just remove memmove_falkor from second line, there is no need to
> move memcpy_thunderx2 below.

Fixed.

> -#define tmp1 x14
> +#define tmp1 x14

> I see no need to move this definition below.

Fixed.

> +#if IS_IN (libc)

> Another non required change, the check is already in place
> before ENTRY_ALIGN.

Fixed.

> +L(copy128):

> Rename it to memcpy_copy128 to make it explicity on memmove code it
> is branching to memcpy or at least add comment stating it.

There is already a comment just above the code that jumps back to memcpy:

+
+   For small and medium cases memcpy is used.  */
+

I'm not sure how much more would need to be said given this is exactly what
we do in the other memcpy implementations, and it is the main goal of this
patch.

> -     /* Align SRC to 16 bytes and copy; that way at least one of the
> +     /* Align src to 16 bytes and copy; that way at least one of the
>           accesses is aligned throughout the copy sequence.

> I see no point in lowercase the SRC here.

Fixed.

> +     .p2align 4
> +     nop
>  L(copy_long):

> If the idea is to align the L(last64), I think is better to add
> .p2align 4 before instead of trying to align here.

Last64 is an unused label, so I removed it. I added a comment to
explain it aligns loop64.

Committed as d1f75e964484504e4f30f4623569d5889a97ac18

Cheers,
Wilco


More information about the Libc-alpha mailing list