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/dcf5d237fd84bec7f202cf418036208b5c9901c9

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 <[email protected]>:
> > 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 <[email protected]>
> > > Signed-off-by: m fally <[email protected]>
> > > ---
> > >  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--)
> > 
> >