On Tue, Oct 14, 2025 at 11:04:59AM +0800, Gary Lin wrote: > 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.
OK, so please put this story into the commit message... Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
