This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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.

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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]