Re: [ARM] Reorganize memcpy selection.

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

On 05/11/15 09:55, Marcus Shawcroft wrote:
> 
> Following up on Richard's comments in
> https://sourceware.org/ml/newlib/2015/msg00739.html this is the first of
> a series of patches to clean up the auto configury mechanism used to
> select different implementations of common functions for various
> architecture versions.
> 
> There are three functions with special handling in Makefile.am, this
> patch addresses only memcpy, following patches will address memchr and
> strlen in a similar fashion.
> 
> The approach here is to remove the selection of memcpy within automake
> and instead use complimentary logic in memcpy-stub.c and memcpy.S to
> choose between the generic memcpy.c implemenation or one of the
> architecture specific memcpy*.S implemenations.
> 
> If this patch is taken I'll followup with equivalent rework for memchr
> and strlen.
> 
> Regressed for armv7-a armv5 armv8-a, correct selection of memcpy
> implementation by manual inspection of a test program built for these
> three architectures.
> 
> OK ?
> 
> 
> /Marcus
> 
> 
> 2015-11-05  Marcus Shawcroft  <marcus.shawcroft@arm.com>
> 
>        * libc/machine/arm/Makefile.am: Drop MEMCPY_SRC and MEMCPY_OBJ.
>        * libc/machine/arm/Makefile.in: Regenerate.
>        * libc/machine/arm/memcpy-stub.c: New.
>        * libc/machine/arm/memcpy.c: Adjust copyright year.  Adjust
>          comments.
> 
> 0002-ARM-Reorganize-memcpy-selection.patch
> 
> 
> diff --git a/newlib/ChangeLog b/newlib/ChangeLog
> index bbb4683..2842389 100644
> --- a/newlib/ChangeLog
> +++ b/newlib/ChangeLog
> @@ -1,3 +1,10 @@
> +2015-11-03  Marcus Shawcroft  <marcus.shawcroft@arm.com>
> +
> +	* libc/machine/arm/Makefile.am: Drop MEMCPY_SRC and MEMCPY_OBJ.
> +	* libc/machine/arm/Makefile.in: Regenerate.
> +	* libc/machine/arm/memcpy-stub.c: New.
> +	* libc/machine/arm/memcpy.c: Adjust copyright year.  Adjust comments.
> +
>  2015-11-04  Marcus Shawcroft  <marcus.shawcroft@arm.com>
>  
>  	* libc/machine/arm/configure.in (HAVE_ARMV7): Correct logic.

> diff --git a/newlib/libc/machine/arm/memcpy-stub.c b/newlib/libc/machine/arm/memcpy-stub.c
> new file mode 100644
> index 0000000..c9de7c9
> --- /dev/null
> +++ b/newlib/libc/machine/arm/memcpy-stub.c
[...]
> +/* The structure of the following #if #else #endif conditional chain
> +   must match the chain in memcpy.S.  */
> +
> +#if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED))
> +# include "../../string/memcpy.c"
> +#elif (__ARM_ARCH >= 7 && __ARM_ARCH_PROFILE == 'A' \
> +       && defined (__ARM_FEATURE_UNALIGNED))

You should include acle-compat.h before testing ACLE macros, so that
this will work with older GCC.

> +/* Defined in memcpy.S.  */
> +#elif defined (__ARM_ARCH_7M__) || defined (__ARM_ARCH_7EM__)

Ideally these should also be replaced with ACLE style tests.

> +/* Defined in memcpy.S.  */
> +#else
> +# include "../../string/memcpy.c"
> +#endif
> diff --git a/newlib/libc/machine/arm/memcpy.S b/newlib/libc/machine/arm/memcpy.S
> index b1bab88..854796a 100644
> --- a/newlib/libc/machine/arm/memcpy.S
> +++ b/newlib/libc/machine/arm/memcpy.S
> @@ -26,19 +26,11 @@
>   * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  

Similarly.

OK with those changes.

R.