Re: [musl] [PATCH v2] Add big-endian support to ARM assembler memcpy

[Andre McCurdy <armccurdy@gmail.com>]

On Thu, Jun 25, 2020 at 3:06 PM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Jan 21, 2020 at 10:52:15AM -0800, Andre McCurdy wrote:
> > Allow the existing ARM assembler memcpy implementation to be used for
> > both big and little endian targets.
> > ---
> >
> > Exactly the same changes as before but rebased to account for
> > whitespace changes in the preceding patch to add Thumb2 support.
> >
> >  COPYRIGHT                                |   2 +-
> >  src/string/arm/{memcpy_le.S => memcpy.S} | 101 ++++++++++++++++++++++-
> >  src/string/arm/memcpy.c                  |   3 -
> >  3 files changed, 98 insertions(+), 8 deletions(-)
> >  rename src/string/arm/{memcpy_le.S => memcpy.S} (82%)
> >  delete mode 100644 src/string/arm/memcpy.c
> >
> > diff --git a/COPYRIGHT b/COPYRIGHT
> > index e6472371..d3edc2a2 100644
> > --- a/COPYRIGHT
> > +++ b/COPYRIGHT
> > @@ -127,7 +127,7 @@ Copyright © 2017-2018 Arm Limited
> >  and labelled as such in comments in the individual source files. All
> >  have been licensed under extremely permissive terms.
> >
> > -The ARM memcpy code (src/string/arm/memcpy_el.S) is Copyright © 2008
> > +The ARM memcpy code (src/string/arm/memcpy.S) is Copyright © 2008
> >  The Android Open Source Project and is licensed under a two-clause BSD
> >  license. It was taken from Bionic libc, used on Android.
> >
> > diff --git a/src/string/arm/memcpy_le.S b/src/string/arm/memcpy.S
> > similarity index 82%
> > rename from src/string/arm/memcpy_le.S
> > rename to src/string/arm/memcpy.S
> > index 7b35d305..869e3448 100644
> > --- a/src/string/arm/memcpy_le.S
> > +++ b/src/string/arm/memcpy.S
> > @@ -1,5 +1,3 @@
> > -#if !__ARMEB__
> > -
> >  /*
> >   * Copyright (C) 2008 The Android Open Source Project
> >   * All rights reserved.
> > @@ -42,7 +40,7 @@
> >   * code safely callable from thumb mode, adjusting the return
> >   * instructions to be compatible with pre-thumb ARM cpus, removal of
> >   * prefetch code that is not compatible with older cpus and support for
> > - * building as thumb 2.
> > + * building as thumb 2 and big-endian.
> >   */
> >
> >  .syntax unified
> > @@ -227,24 +225,45 @@ non_congruent:
> >        * becomes aligned to 32 bits (r5 = nb of words to copy for alignment)
> >        */
> >       movs    r5, r5, lsl #31
> > +
> > +#if __ARMEB__
> > +     movmi   r3, r3, ror #24
> > +     strbmi  r3, [r0], #1
> > +     movcs   r3, r3, ror #24
> > +     strbcs  r3, [r0], #1
> > +     movcs   r3, r3, ror #24
> > +     strbcs  r3, [r0], #1
> > +#else
> >       strbmi r3, [r0], #1
> >       movmi   r3, r3, lsr #8
> >       strbcs r3, [r0], #1
> >       movcs   r3, r3, lsr #8
> >       strbcs r3, [r0], #1
> >       movcs   r3, r3, lsr #8
> > +#endif
> >
> >       cmp     r2, #4
> >       blo     partial_word_tail
> >
> > +#if __ARMEB__
> > +     mov     r3, r3, lsr r12
> > +     mov     r3, r3, lsl r12
> > +#endif
> > +
> >       /* Align destination to 32 bytes (cache line boundary) */
> >  1:      tst     r0, #0x1c
> >       beq     2f
> >       ldr     r5, [r1], #4
> >       sub     r2, r2, #4
> > +#if __ARMEB__
> > +     mov     r4, r5,                 lsr lr
> > +     orr     r4, r4, r3
> > +     mov     r3, r5,                 lsl r12
> > +#else
> >       mov     r4, r5,                 lsl lr
> >       orr     r4, r4, r3
> >       mov     r3, r5,                 lsr r12
> > +#endif
>
> Am I missing something or are both cases identical here? That would
> either indicate this is gratuitous or there's a bug here and they were
> intended not to be the same.

Difference here and below is lsr (logical shift right) -vs- lsl
(logical shift left).

> > [...]
> > @@ -350,9 +429,15 @@ less_than_thirtytwo:
> >
> >  1:      ldr     r5, [r1], #4
> >       sub     r2, r2, #4
> > +#if __ARMEB__
> > +     mov     r4, r5,                 lsr lr
> > +     orr     r4, r4, r3
> > +     mov     r3,     r5,                     lsl r12
> > +#else
> >       mov     r4, r5,                 lsl lr
> >       orr     r4, r4, r3
> >       mov     r3,     r5,                     lsr r12
> > +#endif
>
> And again here.
>
> Rich