RE: Update newlib to support efficient string operation functions for Thumb.

["Hale Wang" <Hale dot Wang at arm dot com>]

Hi Joel,


> -----Original Message-----
> From: newlib-owner@sourceware.org [mailto:newlib-
> owner@sourceware.org] On Behalf Of Joel Sherrill
> Sent: 2014å9æ3æ 0:28
> To: Jeff Johnston; newlib@sourceware.org
> Subject: Re: Update newlib to support efficient string operation functions for
> Thumb.
> 
> 
> On 9/2/2014 11:04 AM, Jeff Johnston wrote:
> > ----- Original Message -----
> >> From: "Corinna Vinschen" <vinschen@redhat.com>
> >> To: newlib@sourceware.org
> >> Sent: Tuesday, September 2, 2014 5:05:36 AM
> >> Subject: Re: Update newlib to support efficient string operation functions
> for Thumb.
> >>
> >> Hi Hale,
> >>
> >> On Aug 26 17:44, Hale Wang wrote:
> >>> Hi,
> >>>
> >>> This patch is used to resubmit the previous newlib patch. And at the
> >>> same time, this patch can also clean up the arm backend in newlib.
> >>>
> >>> The memchr-stub.c and memcpy-stub.c are redundant. There is no need
> >>> to explicitely include the corresponding source files(../../string/*.c).
> >>>
> >>> Jeff Johnston provided a cleaner way to realize this. This patch use
> >>> this solution to clean up all the functions in newlib/libc/machine/arm.
> >>>
> >>> By the way, the previous issue fixed in the previous patch is also
> >>> included in this patch.
> >> The patch looks ok, but I have two comments/questions:
> >>
> >>> --- a/newlib/libc/machine/arm/Makefile.am
> >>> +++ b/newlib/libc/machine/arm/Makefile.am
> >>> @@ -6,13 +6,60 @@ INCLUDES = $(NEWLIB_CFLAGS) $(CROSS_CFLAGS)
> >>> $(TARGET_CFLAGS)
> >>>
> >>>  AM_CCASFLAGS = $(INCLUDES)
> >>>
> >>> +if HAVE_THUMB1
> >>> +if OPT_SIZE
> >>> +STRCMP_SRC=strcmp.S
> >>> +STRCMP_OBJ=$(lpfx)strcmp.o
> >>> +STRLEN_SRC=strlen.c
> >>> +STRLEN_OBJ=$(lpfx)strlen.o
> >>> +else
> >>> +STRCMP_SRC=
> >>> +STRCMP_OBJ=
> >>> +STRLEN_SRC=
> >>> +STRLEN_OBJ=
> >>> +endif
> >>> +else
> >>> +STRCMP_SRC=strcmp.S
> >>> +STRCMP_OBJ=$(lpfx)strcmp.o
> >>> +STRLEN_SRC=strlen.c
> >>> +STRLEN_OBJ=$(lpfx)strlen.o
> >>> +endif
> >>> +
> >>> +if HAVE_ARMV7
> >>> +MEMCHR_SRC=memchr.S
> >>> +MEMCHR_OBJ=$(lpfx)memchr.o
> >>> +else
> >>> +MEMCHR_SRC=
> >>> +MEMCHR_OBJ=
> >>> +endif
> >>> +
> >>> +if OPT_SIZE
> >>> +MEMCPY_SRC=
> >>> +MEMCPY_OBJ=
> >>> +else
> >>> +if HAVE_ARMV7A
> >>> +MEMCPY_SRC=memcpy.S
> >>> +MEMCPY_OBJ=$(lpfx)memcpy.o
> >>> +else
> >>> +if HAVE_ARMV7M
> >>> +MEMCPY_SRC=memcpy.S
> >>> +MEMCPY_OBJ=$(lpfx)memcpy.o
> >>> +else
> >>> +MEMCPY_SRC=
> >>> +MEMCPY_OBJ=
> >>> +endif !HAVE_ARMV7M
> >>> +endif !HAVE_ARMV7A
> >>> +endif !OPT_SIZE
> >> Your configure script sets the variables HAVE_THUMB1, OPT_SIZE,
> >> HAVE_ARMV7, HAVE_ARMV7A, and HAVE_ARMV7M for the sake of
> configuration.
> >> But the actual configuration is done here in Makefile.am, rather than
> >> in configure.in.  These variables are not used anywhere else, for
> >> instance, in the sources.
> >>
> >> What you really need in Makefile.am are only the variables
> >> STRCMP_SRC/OBJ, STRLEN_SRC/OBJ, MEMCHR_SRC/OBJ and
> MEMCPY_SRC/OBJ.
> >>
> >> So, instead of defining the HAVE_foo vars in configure.in just to set
> >> the SRC/LIB vars in Makefile.am, wouldn't it make more sense to set
> >> the SRC/OBJ variables directly in configure.in rather than to spread
> >> this configuration over two file?
> > I specified it in my sample.  The whole reason this is being done is
> > to exclude and include files based on the processor and compilation
> > options.  IMO, it is a little clearer to have the reasoning in the
> > Makefile.am where you are piecing together which files are
> > included/excluded but your way does get to use boolean operators which
> makes it much more concise.
> >
> >>> diff --git a/newlib/libc/machine/arm/configure.in
> >>> b/newlib/libc/machine/arm/configure.in
> >>> index 6236338..edf9222 100644
> >>> --- a/newlib/libc/machine/arm/configure.in
> >>> +++ b/newlib/libc/machine/arm/configure.in
> >>> @@ -10,5 +10,133 @@ AC_CONFIG_AUX_DIR(../../../..)
> >>>
> >>>  NEWLIB_CONFIGURE(../../..)
> >>>
> >>> +dnl Check for Thumb1 supported.
> >>> +AC_CACHE_CHECK(whether we are using thumb1,
> >>> +	       acnewlib_cv_thumb1_processor, [dnl cat > conftest.c <<EOF
> >>> +
> >>> +#if defined (__thumb__) && !defined (__thumb2__)
> >>> +  #define _THUMB1
> >>> + #else
> >>> +  #error "not thumb1"
> >>> +#endif
> >>> +int main () {
> >>> +  return 0;
> >>> +}
> >>> +EOF
> >>> +if AC_TRY_COMMAND([${CC} $CFLAGS $CPPFLAGS -c -o conftest.o
> conftest.c
> >>> +
> 	1>&AS_MESSAGE_LOG_FD])
> >>> +then
> >>> +  acnewlib_cv_thumb1_processor=yes; else
> >>> +  acnewlib_cv_thumb1_processor=no;
> >>> +fi
> >>> +rm -f conftest*])
> >>> [...]
> >> Here I was wondering, do you really have to check for thumb1, armv7,
> >> armv7a, armv7m by running conftests?  Isn't this information easily
> >> available from the target triplet?  Sorry if that's a dumb question,
> >> but I'm not in ARM development so I really don't know.
> >>
> > Thumb is specified via -mthumb and is a multilib in gcc.  I don't know
> > if thumb is even used in configuration triplets anymore.  AFAICT it
> > looks like the arches may not always be used in triplets either, looking at the
> arm toolsets out there.
> RTEMS has only one case where were use an odd submodel in the target
> name. And that's gdb to get a specifical simulator variant built.
> 
> For binutils, gcc, newlib, and gdb in all other cases, we use a single target
> name (e.g. arm-rtems) for all multilib options.
> 
> I assume that all CPU-elf no OS configurations are similar since the CPU-RTEMS
> targets are normally very close.
> 
> Please don't break this.
> 

I have done some tests for multilib option(arm-none-eabi). These configurations are correct and the object files are generated within our expectation.
So I think we don't break this, is it correct?

Thanks.
Hale Wang

> --joel
> > -- Jeff J.
> >
> >> Thanks
> >> Corinna
> >>
> >> --
> >> Corinna Vinschen
> >> Cygwin Maintainer
> >> Red Hat
> >>
> 
> --
> Joel Sherrill, Ph.D.             Director of Research & Development
> joel.sherrill@OARcorp.com        On-Line Applications Research
> Ask me about RTEMS: a free RTOS  Huntsville AL 35805
> Support Available                (256) 722-9985
> 
>