When you fix minor issues for earlier patches then you can add my RB to all of them except this one...
On Tue, Sep 02, 2025 at 03:18:05PM +0800, Gary Lin via Grub-devel wrote: > When both "dest" and "src" are aligned, copying the data in chunks > (unsigned long) is more efficient than a byte-by-byte copy. > > Also tweak '__aeabi_memcpy()', '__aeabi_memcpy4()', and > '__aeabi_memcpy8()', since 'grub_memcpy()' is not inline anymore. > > Signed-off-by: Gary Lin <[email protected]> > --- > grub-core/kern/compiler-rt.c | 8 ++++---- > grub-core/kern/misc.c | 30 ++++++++++++++++++++++++++++++ > include/grub/misc.h | 8 +------- > 3 files changed, 35 insertions(+), 11 deletions(-) > > diff --git a/grub-core/kern/compiler-rt.c b/grub-core/kern/compiler-rt.c > index eda689a0c..8f3865e95 100644 > --- a/grub-core/kern/compiler-rt.c > +++ b/grub-core/kern/compiler-rt.c > @@ -24,7 +24,7 @@ > void * GRUB_BUILTIN_ATTR > memcpy (void *dest, const void *src, grub_size_t n) > { > - return grub_memmove (dest, src, n); > + return grub_memcpy (dest, src, n); > } > void * GRUB_BUILTIN_ATTR > memmove (void *dest, const void *src, grub_size_t n) > @@ -372,11 +372,11 @@ grub_int32_t > __aeabi_idiv (grub_int32_t a, grub_int32_t b) > __attribute__ ((alias ("__divsi3"))); > void *__aeabi_memcpy (void *dest, const void *src, grub_size_t n) > - __attribute__ ((alias ("grub_memcpy"))); > + __attribute__ ((alias ("memcpy"))); > void *__aeabi_memcpy4 (void *dest, const void *src, grub_size_t n) > - __attribute__ ((alias ("grub_memcpy"))); > + __attribute__ ((alias ("memcpy"))); > void *__aeabi_memcpy8 (void *dest, const void *src, grub_size_t n) > - __attribute__ ((alias ("grub_memcpy"))); > + __attribute__ ((alias ("memcpy"))); > void *__aeabi_memset (void *s, int c, grub_size_t n) > __attribute__ ((alias ("memset"))); > > diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c > index 2b7922393..016932583 100644 > --- a/grub-core/kern/misc.c > +++ b/grub-core/kern/misc.c > @@ -99,6 +99,36 @@ grub_memmove (void *dest, const void *src, grub_size_t n) > return dest; > } > > +static void * > +__memcpy_aligned (void *dest, const void *src, grub_size_t n) > +{ > + unsigned long *dw = (unsigned long *) dest; > + const unsigned long *sw = (const unsigned long *) src; I think you should use grub_addr_t instead of unsigned long here. > + grub_uint8_t *d; > + const grub_uint8_t *s; > + > + for (; n >= sizeof (unsigned long); n -= sizeof (unsigned long)) Ditto... > + *dw++ = *sw++; > + > + d = (grub_uint8_t *) dw; > + s = (const grub_uint8_t *) sw; > + for (; n > 0; n--) > + *d++ = *s++; > + > + return dest; > +} > + > +void * > +grub_memcpy (void *dest, const void *src, grub_size_t n) > +{ I think it does not make sense to call __memcpy_aligned() for n smaller than at least sizeof(grub_addr_t). > + /* Check if 'dest' and 'src' are aligned */ > + if (((grub_addr_t) dest & (sizeof (unsigned long) - 1)) == 0 && > + ((grub_addr_t) src & (sizeof (unsigned long) - 1)) == 0) > + return __memcpy_aligned (dest, src, n); I think there is a place for further optimization. I can imagine that if there is the same offset x for src and dest from aligned addresses then we can copy data in bytes up to aligned addresses than switch to sizeof(grub_addr_t) copy and finally, if needed, copy data in bytes up to the end. There is also third case when src and dest offsets are different. Then probably it would make sense to make unaligned reads and aligned writes (this can be strongly depended on targets). However, I am not sure how this would pay off because it will make code more complicated. Though if you are interested in doing this then I suppose you should take a look at functions for unaligned accesses in include/grub/types.h. > + return grub_memmove (dest, src, n); > +} Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
