Re: [PATCH 2/3] arm: Remove optpld macro

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi Pat,

On 12/01/17 04:50, Pat Pannuto wrote:
> LTO can re-order top-level assembly blocks, which can cause this
> macro definition to appear after its use (or not at all), causing
> compilation failures. As the macro has very few uses, simply removing
> it by inlining is a simple fix.
>
> n.b. one of the macro invocations in strlen-stub.c was already
> guarded by the relevant #define, so it is simply converted directly
> to a pld
> ---
>   newlib/libc/machine/arm/arm_asm.h     | 13 -------------
>   newlib/libc/machine/arm/strcpy.c      |  8 ++++++--
>   newlib/libc/machine/arm/strlen-stub.c |  8 +++++---
>   3 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/newlib/libc/machine/arm/arm_asm.h b/newlib/libc/machine/arm/arm_asm.h
> index 1bb5edb23..bf18c0a03 100644
> --- a/newlib/libc/machine/arm/arm_asm.h
> +++ b/newlib/libc/machine/arm/arm_asm.h
> @@ -71,12 +71,6 @@
>   #endif
>   .endm
>   
> -.macro optpld	base, offset=#0
> -#if defined (_ISA_ARM_7)
> -	pld	[\base, \offset]
> -#endif
> -.endm
> -
>   #else
>   asm(".macro  RETURN	cond=\n\t"
>   #if defined (_ISA_ARM_4T) || defined (_ISA_THUMB_1)
> @@ -86,13 +80,6 @@ asm(".macro  RETURN	cond=\n\t"
>   #endif
>       ".endm"
>       );
> -
> -asm(".macro optpld	base, offset=#0\n\t"
> -#if defined (_ISA_ARM_7)
> -    "pld	[\\base, \\offset]\n\t"
> -#endif
> -    ".endm"
> -    );
>   #endif
>   
>   #endif /* ARM_ASM__H */
> diff --git a/newlib/libc/machine/arm/strcpy.c b/newlib/libc/machine/arm/strcpy.c
> index b2e3f5177..b90d5cf73 100644
> --- a/newlib/libc/machine/arm/strcpy.c
> +++ b/newlib/libc/machine/arm/strcpy.c
> @@ -44,7 +44,9 @@ strcpy (char* dst, const char* src)
>     asm (
>   #if !(defined(__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED) || \
>         (defined (__thumb__) && !defined (__thumb2__)))
> -       "optpld	r1\n\t"
> +#ifdef _ISA_ARM_7
> +       "pld	r1\n\t"
> +#endif

Here and in other places in the patch you have a syntax error in the PLD instruction.
The syntax for the pld argument should be in square brackets as it's a memory address like so:
pld [r1].
With your patch the newlib build fails for armv7-a targets.
This patch fixes the build failures.

Tested by making sure the newlib build completes successfully.

Can someone please commit it for me if it's ok for master?

Thanks,
Kyrill

2016-01-26  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    * libc/machine/arm/strcpy.c (strcpy): Fix PLD assembly syntax.
    * libc/machine/arm/strlen-stub.c (strlen): Likewise.

diff --git a/newlib/libc/machine/arm/strcpy.c b/newlib/libc/machine/arm/strcpy.c
index 6f358e4..f1205b9 100644
--- a/newlib/libc/machine/arm/strcpy.c
+++ b/newlib/libc/machine/arm/strcpy.c
@@ -45,7 +45,7 @@ strcpy (char* dst, const char* src)
 #if !(defined(__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED) || \
       (defined (__thumb__) && !defined (__thumb2__)))
 #ifdef _ISA_ARM_7
-       "pld	r1\n\t"
+       "pld	[r1]\n\t"
 #endif
        "eor	r2, r0, r1\n\t"
        "mov	ip, r0\n\t"
@@ -78,7 +78,7 @@ strcpy (char* dst, const char* src)
        ".p2align 2\n"
   "2:\n\t"
 #ifdef _ISA_ARM_7
-       "pld	r1, #8\n\t"
+       "pld	[r1, #8]\n\t"
 #endif
        "ldr	r4, [r1], #4\n\t"
        "sub	r2, r3, "magic1(r5)"\n\t"
diff --git a/newlib/libc/machine/arm/strlen-stub.c b/newlib/libc/machine/arm/strlen-stub.c
index 8f87cba..fc2daf1 100644
--- a/newlib/libc/machine/arm/strlen-stub.c
+++ b/newlib/libc/machine/arm/strlen-stub.c
@@ -59,7 +59,7 @@ strlen (const char* str)
        "addr .req r1\n\t"
 
 #ifdef _ISA_ARM_7
-       "pld r0\n\t"
+       "pld [r0]\n\t"
 #endif
        /* Word-align address */
        "bic	addr, r0, #3\n\t"
@@ -116,7 +116,7 @@ strlen (const char* str)
        /* and 4 more bytes  */
        "addeq	len, len, #4\n\t"
 	/* Unroll the loop a bit.  */
-       "pld	addr, #8\n\t"
+       "pld	[addr, #8]\n\t"
        /*  test (data - 0x01010101)  */
        "ittt	eq\n\t"
        "subeq	r2, data, ip\n\t"