Re: [PATCH 1/5] [ARM] Refactor strlen.c #if nesting.

[Richard Earnshaw <Richard dot Earnshaw at foss dot arm dot com>]

On 13/11/15 12:05, Marcus Shawcroft wrote:
> Hi,
> 
> This patch flattens the condition code selection used in strlen in an
> attempt to make the guarding condition for each alternative
> implementation clearer and to structure the logic in a manner that
> makes it easier to maintain complementary logic between the
> alternative 'C' and assembler implementations.
> 
> Tested by building newlib and comparing libc.a binaries before and
> after for all permutations of:
> 
>   Architectures:
>     armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
>     armv6z armv6kz armv6t2 armv6-m armv6s-m armv7 armv7-a
>     armv7ve armv7-r armv7-m armv7e-m armv8-a iwmmxt iwmmxt2
> 
>   ISAs:
>     thumb arm
> 
>   Optimization Levels:
>     Os O2
> 
>   Excluding:
>     armv6s-m -mthumb
>     armv6-m -mthumb
> 
>     armv6zk -mthumb
>     armv6z -mthumb
>     armv6k -mthumb
>     armv6j -mthumb
> 
> 
>        * libc/machine/arm/strlen.c: Refactor nested #if.
> 
> OK ?
> 

ok.

R.

> /Marcus
> 
> 0001-ARM-Refactor-strlen.c-if-nesting.patch
> 
> 
> diff --git a/newlib/ChangeLog b/newlib/ChangeLog
> index e10f5c2..74ff481 100644
> --- a/newlib/ChangeLog
> +++ b/newlib/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-11-13  Marcus Shawcroft  <marcus.shawcroft@arm.com>
> +
> +	* libc/machine/arm/strlen.c: Refactor nested #if.
> +
>  2015-11-12  Anton Kolesov  <Anton.Kolesov@synopsys.com>
>  
>  	* configure.host: Add ARC support.
> diff --git a/newlib/libc/machine/arm/strlen.c b/newlib/libc/machine/arm/strlen.c
> index 7e59e75..ac5d47d 100644
> --- a/newlib/libc/machine/arm/strlen.c
> +++ b/newlib/libc/machine/arm/strlen.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008 ARM Ltd
> + * Copyright (c) 2008-2015 ARM Ltd
>   * All rights reserved.
>   *
>   * Redistribution and use in source and binary forms, with or without
> @@ -31,32 +31,12 @@
>  #include <string.h>
>  #include <limits.h>
>  
> -#if defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED) || \
> -  (defined (__thumb__) && !defined (__thumb2__))
> -
> -# if !defined (PREFER_SIZE_OVER_SPEED) && !defined (__OPTIMIZE_SIZE__)
> -/* Thumb1 only variant.
> -   If speed is preferred, the strlen() function in ../../string/strlen.c
> -   will be used.
> -
> -   Leave this field blank.  So the strlen() is not defined, and this will
> -   automatically pull in the default C definition of strlen() from
> -   ../../string/strlen.c.  No need to include this file explicitely.
> -   The lib_a-strlen.o will not be generated, so it won't replace the default
> -   lib_a-strlen.o which is generated by ../../string/strlen.c.  See the
> -   commands in configure.in and Makefile.am for more details.
> -
> -   However, if we need to rewrite this function to be more efficient,
> -   we can add the corresponding assembly code into this field and change
> -   the commands in configure.in and Makefile.am to allow the corresponding
> -   lib_a-strlen.o to be generated.
> -*/
> -# else
> +#if defined __OPTIMIZE_SIZE__ || defined PREFER_SIZE_OVER_SPEED
> +#if defined __thumb__ && !defined __thumb2__
>  size_t
>  strlen (const char* str)
>  {
>    int scratch;
> -#if defined (__thumb__) && !defined (__thumb2__)
>    size_t len;
>    asm ("mov	%0, #0\n"
>         "1:\n\t"
> @@ -66,7 +46,13 @@ strlen (const char* str)
>         "bne	1b"
>         : "=&r" (len), "=&r" (scratch) : "r" (str) : "memory", "cc");
>    return len - 1;
> +}
> +
>  #else
> +size_t
> +strlen (const char* str)
> +{
> +  int scratch;
>    const char* end;
>    asm ("1:\n\t"
>         "ldrb	%1, [%0], #1\n\t"
> @@ -74,13 +60,17 @@ strlen (const char* str)
>         "bne	1b"
>         : "=&r" (end), "=&r" (scratch) : "0" (str) : "memory", "cc");
>    return end - str - 1;
> -#endif
>  }
>  #endif
> -#else
>  
> -#if !(defined(_ISA_ARM_7) || defined(__ARM_ARCH_6T2__))
> +#else /* defined __OPTIMIZE_SIZE__ || defined PREFER_SIZE_OVER_SPEED */
> +#if defined __thumb__ && ! defined __thumb2__
> +  /* Implemented in ../../string/strlen.c.  */
> +
> +#elif defined _ISA_ARM_7 || defined __ARM_ARCH_6T2__
> +  /* Implemented in strlen-armv7.S.  */
>  
> +#else
>  size_t __attribute__((naked))
>  strlen (const char* str)
>  {
>