[PATCH 2/5] RISC-V: memmove() speed optimized: Replace macros and use fixed-width types
Christian Herber (OSS)
christian.herber@oss.nxp.com
Tue May 27 15:42:18 GMT 2025
In the commit you mentioned, several RISC-V files were updated with the macros that are also used in this patch.
Can you be specific which functionality you believe will be broken?
Christian
> -----Original Message-----
> From: Alexey Lapshin <alexey.lapshin@espressif.com>
> Sent: Tuesday, 27 May 2025 14:54
> To: marlene.fally@gmail.com
> Cc: newlib@sourceware.org; Christian Herber (OSS)
> <christian.herber@oss.nxp.com>
> Subject: Re: [PATCH 2/5] RISC-V: memmove() speed optimized: Replace
> macros and use fixed-width types
>
> Please take a look at this commit:
> https://github.com/bminor/newlib/commit/dcf5d237fd84bec7f202cf4180362
> 08b5c9901c9
>
> I would like to have --enable-newlib-hw-misaligned-access option in working
> condition. Please don't break the behavior
>
> On Tue, 2025-05-27 at 13:32 +0200, Marlene Fally wrote:
> >
> > [External: This email originated outside Espressif]
> >
> >
> > Hi Alexey,
> >
> > thank you for your comments!
> >
> > > These changes makes newlib ignore "_HAVE_HW_MISALIGNED_ACCESS".
> > >
> >
> > I am happy to change it back to use _HAVE_HW_MISALIGNED_ACCESS if
> that
> > is preferred, but since these changes are only to the risc-v port, and
> > the other functions in this port check for either __riscv_misaligned_fast,
> __riscv_misaligned_slow, and/or __riscv_misaligned_avoid, I figured it would
> make sense to opt for one of these macros.
> > Also, checking for __riscv_misaligned_fast allows for still making use of the
> source-alignment-operation if misaligned accesses are allowed but slow.
> >
> > > What purpose of replacing macros? (I can't see advantages to this).
> > > Is "sizeof(long)" gives something different compared to SZREG ?
> > >
> >
> > The purpose of using SZREG here is to have consistency between all the
> functions in the port.
> > I don't think in this case sizeof(long) would give something different
> > – as far as I'm aware the valid ABIs for risc-v have long be 4 bytes for a 32-
> bit architecture, and 8 bytes for a 64-bit architecture, which matches the
> respective SZREG values.
> >
> > Best,
> > M
> >
> >
> >
> > Op di 27 mei 2025 om 08:04 schreef Alexey Lapshin
> <alexey.lapshin@espressif.com>:
> > > These changes makes newlib ignore "_HAVE_HW_MISALIGNED_ACCESS".
> > >
> > > What purpose of replacing macros? (I can't see advantages to this).
> > > Is "sizeof(long)" gives something different compared to SZREG ?
> > >
> > > On Mon, 2025-05-26 at 17:05 +0200, m fally wrote:
> > > > Remove macros or replace them with static inline functions or
> > > > RISC-V specific macros where applicable.
> > > > Change data types to fixed-width and/or RISC-V specific types.
> > > >
> > > > Reviewed-by: Christian Herber <christian.herber@oss.nxp.com>
> > > > Signed-off-by: m fally <marlene.fally@gmail.com>
> > > > ---
> > > > newlib/libc/machine/riscv/memmove.c | 60
> > > > +++++++++++++++++------------
> > > > 1 file changed, 35 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/newlib/libc/machine/riscv/memmove.c
> > > > b/newlib/libc/machine/riscv/memmove.c
> > > > index b48da0905..2e5c6ca9b 100644
> > > > --- a/newlib/libc/machine/riscv/memmove.c
> > > > +++ b/newlib/libc/machine/riscv/memmove.c
> > > > @@ -14,19 +14,29 @@
> > > > #else
> > > >
> > > > #include "../../string/local.h"
> > > > -#include <_ansi.h>
> > > > +#include "sys/asm.h"
> > > > +#include "xlenint.h"
> > > > #include <limits.h>
> > > > #include <stddef.h>
> > > > #include <string.h>
> > > >
> > > > -/*SUPPRESS 20*/
> > > > +static inline uint8_t
> > > > +__libc_fast_xlen_aligned (void *dst, const void *src) { #if
> > > > +defined(__riscv_misaligned_fast)
> > > > + return 1;
> > > > +#else
> > > > + return !(((uintxlen_t)src & (SZREG - 1)) | ((uintxlen_t)dst &
> > > > +(SZREG - 1))); #endif }
> > > > +
> > > > void *__inhibit_loop_to_libcall
> > > > memmove (void *dst_void, const void *src_void, size_t length)
> > > > {
> > > > - char *dst = dst_void;
> > > > - const char *src = src_void;
> > > > - long *aligned_dst;
> > > > - const long *aligned_src;
> > > > + unsigned char *dst = dst_void;
> > > > + const unsigned char *src = src_void;
> > > > + uintxlen_t *aligned_dst;
> > > > + const uintxlen_t *aligned_src;
> > > >
> > > > if (src < dst && dst < src + length)
> > > > {
> > > > @@ -34,21 +44,21 @@ memmove (void *dst_void, const void *src_void,
> > > > size_t length)
> > > > src += length;
> > > > dst += length;
> > > >
> > > > - if (!TOO_SMALL_LITTLE_BLOCK (length) && !UNALIGNED_X_Y
> > > > (src, dst))
> > > > + if (length >= SZREG && __libc_fast_xlen_aligned (dst, src))
> > > > {
> > > > - aligned_dst = (long *)dst;
> > > > - aligned_src = (long *)src;
> > > > + aligned_dst = (uintxlen_t *)dst;
> > > > + aligned_src = (uintxlen_t *)src;
> > > >
> > > > - /* Copy one long word at a time if possible. */
> > > > - while (!TOO_SMALL_LITTLE_BLOCK (length))
> > > > + /* Copy one uintxlen_t word at a time if possible. */
> > > > + while (length >= SZREG)
> > > > {
> > > > *--aligned_dst = *--aligned_src;
> > > > - length -= LITTLE_BLOCK_SIZE;
> > > > + length -= SZREG;
> > > > }
> > > >
> > > > /* Pick up any residual with a byte copier. */
> > > > - dst = (char *)aligned_dst;
> > > > - src = (char *)aligned_src;
> > > > + dst = (unsigned char *)aligned_dst;
> > > > + src = (unsigned char *)aligned_src;
> > > > }
> > > >
> > > > while (length--)
> > > > @@ -61,31 +71,31 @@ memmove (void *dst_void, const void *src_void,
> > > > size_t length)
> > > > /* Use optimizing algorithm for a non-destructive copy to
> > > > closely
> > > > match memcpy. If the size is small or either SRC or DST
> > > > is unaligned,
> > > > then punt into the byte copy loop. This should be rare.
> > > > */
> > > > - if (!TOO_SMALL_LITTLE_BLOCK (length) && !UNALIGNED_X_Y
> > > > (src, dst))
> > > > + if (length >= SZREG && __libc_fast_xlen_aligned (dst, src))
> > > > {
> > > > - aligned_dst = (long *)dst;
> > > > - aligned_src = (long *)src;
> > > > + aligned_dst = (uintxlen_t *)dst;
> > > > + aligned_src = (uintxlen_t *)src;
> > > >
> > > > - /* Copy 4X long words at a time if possible. */
> > > > - while (!TOO_SMALL_BIG_BLOCK (length))
> > > > + /* Copy 4X uintxlen_t words at a time if possible. */
> > > > + while (length >= (SZREG * 4))
> > > > {
> > > > *aligned_dst++ = *aligned_src++;
> > > > *aligned_dst++ = *aligned_src++;
> > > > *aligned_dst++ = *aligned_src++;
> > > > *aligned_dst++ = *aligned_src++;
> > > > - length -= BIG_BLOCK_SIZE;
> > > > + length -= SZREG * 4;
> > > > }
> > > >
> > > > - /* Copy one long word at a time if possible. */
> > > > - while (!TOO_SMALL_LITTLE_BLOCK (length))
> > > > + /* Copy one uintxlen_t word at a time if possible. */
> > > > + while (length >= SZREG)
> > > > {
> > > > *aligned_dst++ = *aligned_src++;
> > > > - length -= LITTLE_BLOCK_SIZE;
> > > > + length -= SZREG;
> > > > }
> > > >
> > > > /* Pick up any residual with a byte copier. */
> > > > - dst = (char *)aligned_dst;
> > > > - src = (char *)aligned_src;
> > > > + dst = (unsigned char *)aligned_dst;
> > > > + src = (unsigned char *)aligned_src;
> > > > }
> > > >
> > > > while (length--)
> > >
> > >
More information about the Newlib
mailing list