Re: [PATCH, newlib, ping1] Fix strlen using Thumb-2 with -Os -marm

[Jeff Johnston <jjohnstn at redhat dot com>]

Patch applied.

-- Jeff J.

----- Original Message -----
> Ping?
> 
> On Thursday 28 April 2016 17:06:45 Thomas Preudhomme wrote:
> > Hi,
> > 
> > Currently, the selection of which strlen routine to use for ARM processors
> > is as follows (see newlib/libc/machine/arm/strlen.S or
> > newlib/libc/machine/arm/strlen-stub.c):
> > 
> > #if optimize for size
> > #if defined __thumb__ && !defined __thumb2__
> > Thumb-1 routine
> > #else
> > Thumb-2 routine
> > #endif
> > #else
> > something
> > #endif
> > 
> > When compiling for ARM ISA, __thumb__ would be 0 and thus the Thumb-2
> > routine would be chosen, no matter the architecture selected. This patch
> > changes the logic to only look for capabilities of the processor. It also
> > fallback to the C implementation for targets with only ARM execution state
> > (armv4 and armv5).
> > 
> > ChangeLog entry is as follows:
> > 
> > *** newlib/ChangeLog ***
> > 
> > 2016-04-18  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >         * libc/machine/arm/strlen-stub.c: Check capabilities of
> >         architecture
> > to decide which Thumb implementation to use and fall back to C
> > implementation for architecture not supporting Thumb mode. *
> > libc/machine/arm/strlen.S: Likewise.
> > 
> > 
> > diff --git a/newlib/libc/machine/arm/strlen-stub.c
> > b/newlib/libc/machine/arm/strlen-stub.c
> > index
> > bcd3d2d462587f6daa4a96aabf9f773962435190..ea45a378991011ab77af697c22c79fd2cb
> > 957d02 100644
> > --- a/newlib/libc/machine/arm/strlen-stub.c
> > +++ b/newlib/libc/machine/arm/strlen-stub.c
> > @@ -32,12 +32,15 @@
> >  #include <limits.h>
> > 
> >  #if defined __OPTIMIZE_SIZE__ || defined PREFER_SIZE_OVER_SPEED
> > -#if defined __thumb__ && !defined __thumb2__
> > +#if __ARM_ARCH_ISA_THUMB == 2
> >  /* Implemented in strlen.S.  */
> > 
> > -#else
> > +#elif defined (__ARM_ARCH_ISA_THUMB)
> >  /* Implemented in strlen.S.  */
> > 
> > +#else
> > +#include "../../string/strlen.c"
> > +
> >  #endif
> > 
> >  #else /* defined __OPTIMIZE_SIZE__ || defined PREFER_SIZE_OVER_SPEED */
> > diff --git a/newlib/libc/machine/arm/strlen.S
> > b/newlib/libc/machine/arm/strlen.S
> > index
> > 57371453a5a330aef5c02836eb8d3a1ae446294e..0435fb2de8489380e28f554dcd66ec6cb5
> > 744d75 100644
> > --- a/newlib/libc/machine/arm/strlen.S
> > +++ b/newlib/libc/machine/arm/strlen.S
> > @@ -27,11 +27,14 @@
> >  #include "acle-compat.h"
> > 
> >  #if defined __OPTIMIZE_SIZE__ || defined PREFER_SIZE_OVER_SPEED
> > -#if defined __thumb__ && !defined __thumb2__
> > +#if __ARM_ARCH_ISA_THUMB == 2
> > +#include "strlen-thumb2-Os.S"
> > +
> > +#elif defined (__ARM_ARCH_ISA_THUMB)
> >  #include "strlen-thumb1-Os.S"
> > 
> >  #else
> > -#include "strlen-thumb2-Os.S"
> > +  /* Implemented in strlen-stub.c.  */
> > 
> >  #endif
> > 
> > 
> > 
> > Tested by building newlib and comparing all *.a and *.o binaries before and
> > after with objdump -D and readelf -A for all permutations of:
> > 
> >   Architectures:
> >     armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
> >     armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
> >     armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
> >     iwmmxt2
> > 
> >   ISAs:
> >     thumb arm
> > 
> >   Optimization Levels:
> >     Os O2
> > 
> >   Excluding:
> >     armv6s-m -marm
> >     armv7 -marm
> >     armv7-m -marm
> >     armv7e-m -marm
> >     iwmmxt -mthumb
> >     iwmmxt2 -mthumb
> > 
> > as being rejected by the compiler or assembler. ARMv6-M was not tested
> > because compilation fails due to svc instruction. The newlib built was
> > configured with --disable-newlib-supplied-syscalls.
> > 
> > The comparison showed that the Thumb-1 implementation is used instead of
> > Thumb-2 for all of: armv4t armv5 armv5t armv5te armv6 armv6j armv6k armv6kz
> > armv6z armv6zk iwmmxt iwmmxt2. Note that armv5 and iWMMXt are in that list
> > due to a bug in GCC in the definition of TARGET_ARM_ARCH_ISA_THUMB. It also
> > shows that armv4 now uses the C implementation. Other combinations (-O2,
> > -mthumb and Thumb-2 capable architectures) are not affected.
> > 
> > Is this ok for master branch?
> > 
> > Best regards,
> > 
> > Thomas
> 
>