On Mon, Oct 13, 2025 at 05:17:33PM +0200, Daniel Kiper wrote: > When you fix minor issues for earlier patches then you can add my RB to > all of them except this one... > Thanks!
> 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. > Will fix it in v5. > > + 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). > Right. I'll fix it in v5. > > + /* 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. > I actually tried to imitate memcpy() in glibc but found it's complex to maintain. Since __memcpy_aligned() already brings some noticeable improvements, I decided to keep it simple, as least for this patch set. Gary Lin > > + return grub_memmove (dest, src, n); > > +} > > Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
