public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 2/3] arm: Remove optpld macro
  2017-01-12  4:51 [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
  2017-01-12  4:51 ` [PATCH 1/3] Remove unneeded references to arm_asm.h Pat Pannuto
  2017-01-12  4:51 ` [PATCH 3/3] arm: Remove RETURN macro Pat Pannuto
@ 2017-01-12  4:51 ` Pat Pannuto
  2017-01-12 14:56   ` Corinna Vinschen
                     ` (2 more replies)
  2017-01-20 20:06 ` [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
  2017-01-25 12:32 ` Corinna Vinschen
  4 siblings, 3 replies; 13+ messages in thread
From: Pat Pannuto @ 2017-01-12  4:51 UTC (permalink / raw)
  To: newlib; +Cc: Pat Pannuto

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
        "eor	r2, r0, r1\n\t"
        "mov	ip, r0\n\t"
        "tst	r2, #3\n\t"
@@ -75,7 +77,9 @@ strcpy (char* dst, const char* src)
 	  load stalls.  */
        ".p2align 2\n"
   "2:\n\t"
-       "optpld	r1, #8\n\t"
+#ifdef _ISA_ARM_7
+       "pld	r1, #8\n\t"
+#endif
        "ldr	r4, [r1], #4\n\t"
        "sub	r2, r3, "magic1(r5)"\n\t"
        "bics	r2, r2, r3\n\t"
diff --git a/newlib/libc/machine/arm/strlen-stub.c b/newlib/libc/machine/arm/strlen-stub.c
index ea45a3789..69cfa3df0 100644
--- a/newlib/libc/machine/arm/strlen-stub.c
+++ b/newlib/libc/machine/arm/strlen-stub.c
@@ -58,7 +58,9 @@ strlen (const char* str)
        "data .req r3\n\t"
        "addr .req r1\n\t"
 
-       "optpld r0\n\t"
+#ifdef _ISA_ARM_7
+       "pld r0\n\t"
+#endif
        /* Word-align address */
        "bic	addr, r0, #3\n\t"
        /* Get adjustment for start ... */
@@ -113,8 +115,8 @@ strlen (const char* str)
        "ldreq	data, [addr], #4\n\t"
        /* and 4 more bytes  */
        "addeq	len, len, #4\n\t"
-	/* If we have PLD, then unroll the loop a bit.  */
-       "optpld addr, #8\n\t"
+	/* Unroll the loop a bit.  */
+       "pld	addr, #8\n\t"
        /*  test (data - 0x01010101)  */
        "ittt	eq\n\t"
        "subeq	r2, data, ip\n\t"
-- 
2.11.0

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 3/3] arm: Remove RETURN macro
  2017-01-12  4:51 [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
  2017-01-12  4:51 ` [PATCH 1/3] Remove unneeded references to arm_asm.h Pat Pannuto
@ 2017-01-12  4:51 ` Pat Pannuto
  2017-01-12  4:51 ` [PATCH 2/3] arm: Remove optpld macro Pat Pannuto
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Pat Pannuto @ 2017-01-12  4:51 UTC (permalink / raw)
  To: newlib; +Cc: Pat Pannuto

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. On modern toolchains (armv4t+), assembly
should write `bx lr` in all cases, and linkers will transparently
convert them to `mov pc, lr`, allowing us to simply remove the
macro.
  (source: https://groups.google.com/forum/#!topic/comp.sys.arm/3l7fVGX-Wug
   and verified empirically)

For the armv4.S file, preserve this macro to maximize backwards
compatibility.
---
 newlib/libc/machine/arm/arm_asm.h         | 22 ----------------------
 newlib/libc/machine/arm/strcmp-arm-tiny.S |  2 +-
 newlib/libc/machine/arm/strcmp-armv4.S    | 12 ++++++++++++
 newlib/libc/machine/arm/strcmp-armv7m.S   |  6 +++---
 newlib/libc/machine/arm/strcpy.c          | 12 ++++++------
 newlib/libc/machine/arm/strlen-stub.c     |  2 +-
 6 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/newlib/libc/machine/arm/arm_asm.h b/newlib/libc/machine/arm/arm_asm.h
index bf18c0a03..2708057de 100644
--- a/newlib/libc/machine/arm/arm_asm.h
+++ b/newlib/libc/machine/arm/arm_asm.h
@@ -60,26 +60,4 @@
 # define _ISA_THUMB_1
 #endif
 
-
-/* Now some macros for common instruction sequences.  */
-#ifdef __ASSEMBLER__
-.macro  RETURN     cond=
-#if defined (_ISA_ARM_4T) || defined (_ISA_THUMB_1)
-	bx\cond	lr
-#else
-	mov\cond pc, lr
-#endif
-.endm
-
-#else
-asm(".macro  RETURN	cond=\n\t"
-#if defined (_ISA_ARM_4T) || defined (_ISA_THUMB_1)
-    "bx\\cond	lr\n\t"
-#else
-    "mov\\cond	pc, lr\n\t"
-#endif
-    ".endm"
-    );
-#endif
-
 #endif /* ARM_ASM__H */
diff --git a/newlib/libc/machine/arm/strcmp-arm-tiny.S b/newlib/libc/machine/arm/strcmp-arm-tiny.S
index 6b6bd13cc..607a41daf 100644
--- a/newlib/libc/machine/arm/strcmp-arm-tiny.S
+++ b/newlib/libc/machine/arm/strcmp-arm-tiny.S
@@ -42,6 +42,6 @@ def_fn strcmp
 	beq	1b
 2:
 	subs	r0, r2, r3
-	RETURN
+	bx	lr
 	.cfi_endproc
 	.size	strcmp, . - strcmp
diff --git a/newlib/libc/machine/arm/strcmp-armv4.S b/newlib/libc/machine/arm/strcmp-armv4.S
index 05e3df675..e8d0e2468 100644
--- a/newlib/libc/machine/arm/strcmp-armv4.S
+++ b/newlib/libc/machine/arm/strcmp-armv4.S
@@ -43,6 +43,18 @@
 #define tmp1		r12
 #define syndrome	r12	/* Overlaps tmp1 */
 
+/* For armv4t and newer, toolchains will transparently convert
+   'bx lr' to 'mov pc, lr' if needed. GCC has deprecated support
+   for anything older than armv4t, but this should handle that
+   corner case in case anyone needs it anyway */
+.macro  RETURN
+#if __ARM_ARCH <= 4 && __ARM_ARCH_ISA_THUMB == 0
+	mov	pc, lr
+#else
+	bx	lr
+#endif
+.endm
+
 	.arm
 def_fn strcmp
 	.cfi_sections .debug_frame
diff --git a/newlib/libc/machine/arm/strcmp-armv7m.S b/newlib/libc/machine/arm/strcmp-armv7m.S
index 7b6304925..cdb4912df 100644
--- a/newlib/libc/machine/arm/strcmp-armv7m.S
+++ b/newlib/libc/machine/arm/strcmp-armv7m.S
@@ -106,7 +106,7 @@ def_fn strcmp
 	lsrs	result, result, #24
 	subs	result, result, data2
 #endif
-	RETURN
+	bx	lr
 
 
 #if 0
@@ -356,7 +356,7 @@ def_fn strcmp
 	ldmfd	sp!, {r5}
 	.cfi_restore 5
 	.cfi_def_cfa_offset 0
-	RETURN
+	bx	lr
 
 .Lstrcmp_tail:
 	.cfi_restore_state
@@ -373,6 +373,6 @@ def_fn strcmp
 	ldmfd	sp!, {r5}
 	.cfi_restore 5
 	.cfi_def_cfa_offset 0
-	RETURN
+	bx	lr
 	.cfi_endproc
 	.size strcmp, . - strcmp
diff --git a/newlib/libc/machine/arm/strcpy.c b/newlib/libc/machine/arm/strcpy.c
index b90d5cf73..6f358e489 100644
--- a/newlib/libc/machine/arm/strcpy.c
+++ b/newlib/libc/machine/arm/strcpy.c
@@ -108,7 +108,7 @@ strcpy (char* dst, const char* src)
 #ifndef __thumb2__
        "ldr	r5, [sp], #4\n\t"
 #endif
-       "RETURN\n"
+       "bx	lr\n"
 
        /* Strings have the same offset from word alignment, but it's
 	  not zero.  */
@@ -119,7 +119,7 @@ strcpy (char* dst, const char* src)
        "strb	r2, [ip], #1\n\t"
        "cmp	r2, #0\n\t"
        "it	eq\n"
-       "RETURN	eq\n"
+       "bxeq	lr\n"
   "1:\n\t"
        "tst	r1, #2\n\t"
        "beq	5b\n\t"
@@ -139,7 +139,7 @@ strcpy (char* dst, const char* src)
        "tstne	r2, #0xff00\n\t"
 #endif
        "bne	5b\n\t"
-       "RETURN\n"
+       "bx	lr\n"
 
        /* src and dst do not have a common word-alignement.  Fall back to
 	  byte copying.  */
@@ -148,7 +148,7 @@ strcpy (char* dst, const char* src)
        "strb	r2, [ip], #1\n\t"
        "cmp	r2, #0\n\t"
        "bne	4b\n\t"
-       "RETURN"
+       "bx	lr\n\t"
 
 #elif !defined (__thumb__) || defined (__thumb2__)
        "mov	r3, r0\n\t"
@@ -157,7 +157,7 @@ strcpy (char* dst, const char* src)
        "strb	r2, [r3], #1\n\t"
        "cmp	r2, #0\n\t"
        "bne	1b\n\t"
-       "RETURN"
+       "bx	lr\n\t"
 #else
        "mov	r3, r0\n\t"
   "1:\n\t"
@@ -167,7 +167,7 @@ strcpy (char* dst, const char* src)
        "add	r3, r3, #1\n\t"
        "cmp	r2, #0\n\t"
        "bne	1b\n\t"
-       "RETURN"
+       "bx	lr\n\t"
 #endif
        );
 }
diff --git a/newlib/libc/machine/arm/strlen-stub.c b/newlib/libc/machine/arm/strlen-stub.c
index 69cfa3df0..8f87cba09 100644
--- a/newlib/libc/machine/arm/strlen-stub.c
+++ b/newlib/libc/machine/arm/strlen-stub.c
@@ -168,7 +168,7 @@ strlen (const char* str)
        "addne	len, len, #1\n\t"
 # endif
 #endif
-       "RETURN");
+       "bx	lr\n\t");
 }
 #endif
 #endif
-- 
2.11.0

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 1/3] Remove unneeded references to arm_asm.h
  2017-01-12  4:51 [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
@ 2017-01-12  4:51 ` Pat Pannuto
  2017-01-12  4:51 ` [PATCH 3/3] arm: Remove RETURN macro Pat Pannuto
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Pat Pannuto @ 2017-01-12  4:51 UTC (permalink / raw)
  To: newlib; +Cc: Pat Pannuto

This should result in no functional changes, it simply removes references
to arm_asm.h that did not use anything from that file.
---
 newlib/libc/machine/arm/Makefile.am           | 2 +-
 newlib/libc/machine/arm/Makefile.in           | 2 +-
 newlib/libc/machine/arm/aeabi_memcpy-armv7a.S | 2 +-
 newlib/libc/machine/arm/aeabi_memmove-soft.S  | 2 --
 newlib/libc/machine/arm/aeabi_memset-soft.S   | 2 --
 newlib/libc/machine/arm/memchr.S              | 1 -
 newlib/libc/machine/arm/strcmp.S              | 1 -
 newlib/libc/machine/arm/strlen-armv7.S        | 2 +-
 8 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/newlib/libc/machine/arm/Makefile.am b/newlib/libc/machine/arm/Makefile.am
index 62ed158f3..6b6705027 100644
--- a/newlib/libc/machine/arm/Makefile.am
+++ b/newlib/libc/machine/arm/Makefile.am
@@ -25,7 +25,7 @@ lib_a_CFLAGS = $(AM_CFLAGS)
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
 
-MEMCHR_DEP=acle-compat.h arm_asm.h
+MEMCHR_DEP=acle-compat.h
 MEMCPY_DEP=memcpy-armv7a.S memcpy-armv7m.S
 STRCMP_DEP=strcmp-arm-tiny.S strcmp-armv4.S strcmp-armv4t.S strcmp-armv6.S \
 	strcmp-armv6m.S strcmp-armv7.S strcmp-armv7m.S
diff --git a/newlib/libc/machine/arm/Makefile.in b/newlib/libc/machine/arm/Makefile.in
index 49082088f..a0e4cc2bd 100644
--- a/newlib/libc/machine/arm/Makefile.in
+++ b/newlib/libc/machine/arm/Makefile.in
@@ -215,7 +215,7 @@ lib_a_CCASFLAGS = $(AM_CCASFLAGS)
 lib_a_CFLAGS = $(AM_CFLAGS)
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
-MEMCHR_DEP = acle-compat.h arm_asm.h
+MEMCHR_DEP = acle-compat.h
 MEMCPY_DEP = memcpy-armv7a.S memcpy-armv7m.S
 STRCMP_DEP = strcmp-arm-tiny.S strcmp-armv4.S strcmp-armv4t.S strcmp-armv6.S \
 	strcmp-armv6m.S strcmp-armv7.S strcmp-armv7m.S
diff --git a/newlib/libc/machine/arm/aeabi_memcpy-armv7a.S b/newlib/libc/machine/arm/aeabi_memcpy-armv7a.S
index 53e3330ff..95f2dcd00 100644
--- a/newlib/libc/machine/arm/aeabi_memcpy-armv7a.S
+++ b/newlib/libc/machine/arm/aeabi_memcpy-armv7a.S
@@ -26,7 +26,7 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include "arm_asm.h"
+#include "acle-compat.h"
 
 /* NOTE: This ifdef MUST match the one in aeabi_memcpy.c.  */
 #if defined (__ARM_ARCH_7A__) && defined (__ARM_FEATURE_UNALIGNED) && \
diff --git a/newlib/libc/machine/arm/aeabi_memmove-soft.S b/newlib/libc/machine/arm/aeabi_memmove-soft.S
index 010e266d7..8c071bde0 100644
--- a/newlib/libc/machine/arm/aeabi_memmove-soft.S
+++ b/newlib/libc/machine/arm/aeabi_memmove-soft.S
@@ -26,8 +26,6 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include "arm_asm.h"
-
 .macro	ASM_ALIAS new old
 	.global	\new
 	.type	\new, %function
diff --git a/newlib/libc/machine/arm/aeabi_memset-soft.S b/newlib/libc/machine/arm/aeabi_memset-soft.S
index d82fb5606..cacf2f406 100644
--- a/newlib/libc/machine/arm/aeabi_memset-soft.S
+++ b/newlib/libc/machine/arm/aeabi_memset-soft.S
@@ -26,8 +26,6 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include "arm_asm.h"
-
 .macro	ASM_ALIAS new old
 	.global	\new
 	.type	\new, %function
diff --git a/newlib/libc/machine/arm/memchr.S b/newlib/libc/machine/arm/memchr.S
index 8103c8aa5..ef2d6d08b 100644
--- a/newlib/libc/machine/arm/memchr.S
+++ b/newlib/libc/machine/arm/memchr.S
@@ -76,7 +76,6 @@
 	.syntax unified
 
 #include "acle-compat.h"
-#include "arm_asm.h"
 
 @ NOTE: This ifdef MUST match the one in memchr-stub.c
 #if __ARM_ARCH_ISA_THUMB >= 2 && defined (__ARM_FEATURE_DSP)
diff --git a/newlib/libc/machine/arm/strcmp.S b/newlib/libc/machine/arm/strcmp.S
index 3af9b6dbd..137f44969 100644
--- a/newlib/libc/machine/arm/strcmp.S
+++ b/newlib/libc/machine/arm/strcmp.S
@@ -28,7 +28,6 @@
 
 /* Wrapper for the various implementations of strcmp.  */
 
-#include "arm_asm.h"
 #include "acle-compat.h"
 
 #ifdef __ARM_BIG_ENDIAN
diff --git a/newlib/libc/machine/arm/strlen-armv7.S b/newlib/libc/machine/arm/strlen-armv7.S
index 9dce6f2f7..f3dda0d60 100644
--- a/newlib/libc/machine/arm/strlen-armv7.S
+++ b/newlib/libc/machine/arm/strlen-armv7.S
@@ -58,7 +58,7 @@
    (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
    OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.  */
 
-#include "arm_asm.h"
+#include "acle-compat.h"
 
 	.macro def_fn f p2align=0
 	.text
-- 
2.11.0

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH 0/3] arm: Fix link time optimization issues
@ 2017-01-12  4:51 Pat Pannuto
  2017-01-12  4:51 ` [PATCH 1/3] Remove unneeded references to arm_asm.h Pat Pannuto
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Pat Pannuto @ 2017-01-12  4:51 UTC (permalink / raw)
  To: newlib; +Cc: Pat Pannuto

This patch series fixes a few things so that link time optimization will work.
The high-level problem is that LTO can re-order top-level assembly blocks, so
code that previously worked because one asm() block was listed before another
at global scope in a file (in this case via #include) may no longer work.

So far this was only an issue with two macros that were defined in an #include'd
header. With these patches, I have LTO working with newlib on arm.

Pat Pannuto (3):
  Remove unneeded references to arm_asm.h
  arm: Remove optpld macro
  arm: Remove RETURN macro

 newlib/libc/machine/arm/Makefile.am           |  2 +-
 newlib/libc/machine/arm/Makefile.in           |  2 +-
 newlib/libc/machine/arm/aeabi_memcpy-armv7a.S |  2 +-
 newlib/libc/machine/arm/aeabi_memmove-soft.S  |  2 --
 newlib/libc/machine/arm/aeabi_memset-soft.S   |  2 --
 newlib/libc/machine/arm/arm_asm.h             | 35 ---------------------------
 newlib/libc/machine/arm/memchr.S              |  1 -
 newlib/libc/machine/arm/strcmp-arm-tiny.S     |  2 +-
 newlib/libc/machine/arm/strcmp-armv4.S        | 12 +++++++++
 newlib/libc/machine/arm/strcmp-armv7m.S       |  6 ++---
 newlib/libc/machine/arm/strcmp.S              |  1 -
 newlib/libc/machine/arm/strcpy.c              | 20 +++++++++------
 newlib/libc/machine/arm/strlen-armv7.S        |  2 +-
 newlib/libc/machine/arm/strlen-stub.c         | 10 +++++---
 14 files changed, 38 insertions(+), 61 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
  2017-01-12  4:51 ` [PATCH 2/3] arm: Remove optpld macro Pat Pannuto
@ 2017-01-12 14:56   ` Corinna Vinschen
  2017-01-12 15:56     ` Craig Howland
  2017-01-14  5:24   ` Pat Pannuto
  2017-01-26 10:26   ` Kyrill Tkachov
  2 siblings, 1 reply; 13+ messages in thread
From: Corinna Vinschen @ 2017-01-12 14:56 UTC (permalink / raw)
  To: newlib

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

On Jan 11 23: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.

Am I the only one thinking this is a compiler bug, rather than correct
behaviour?

The macro definition is not an assembly block as such, but rather just
that: a macro definition which is supposed to be evaluated at compile
(or assemble) time.  To reorder it on the source level seems completely
out of the scope of LTO.

Am I missing something or should this be fixed in GCC?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
  2017-01-12 14:56   ` Corinna Vinschen
@ 2017-01-12 15:56     ` Craig Howland
  0 siblings, 0 replies; 13+ messages in thread
From: Craig Howland @ 2017-01-12 15:56 UTC (permalink / raw)
  To: newlib

On 01/12/2017 09:56 AM, Corinna Vinschen wrote:
> On Jan 11 23: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.
> Am I the only one thinking this is a compiler bug, rather than correct
> behaviour?
>
> The macro definition is not an assembly block as such, but rather just
> that: a macro definition which is supposed to be evaluated at compile
> (or assemble) time.  To reorder it on the source level seems completely
> out of the scope of LTO.
>
> Am I missing something or should this be fixed in GCC?
>
>
> Corinna
>
I was asking the same question, so you are not the only one.  (I'm also confused 
because the patch is talking about link-time optimization, not compilation.  But 
Pat says it causes a compilation failure.  So then how is this related to LTO, 
even?)

In addition, I have another question/issue.  Sure, it is easy to modify the 
Newlib code, itself, to avoid this macro, but how can we know that there is not 
any user code which uses it that would now fail when it has been removed?  This 
is a higher-level include file that users might have used; it is not a 
lower-level, newlib-internal file, it does not seem.  That is, is it OK for 
compatibility reasons to just remove it (especially without a note as to why it 
is dangerous)?

Craig

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
  2017-01-12  4:51 ` [PATCH 2/3] arm: Remove optpld macro Pat Pannuto
  2017-01-12 14:56   ` Corinna Vinschen
@ 2017-01-14  5:24   ` Pat Pannuto
       [not found]     ` <33404bdb-e4e3-f77e-7a97-42def0a945ff@t-online.de>
  2017-01-26 10:26   ` Kyrill Tkachov
  2 siblings, 1 reply; 13+ messages in thread
From: Pat Pannuto @ 2017-01-14  5:24 UTC (permalink / raw)
  To: newlib; +Cc: Pat Pannuto

First, sorry to reply to my own e-mail, I wasn't previously subscribed to the
newlib mailing list and got dropped from the CC list, I've quoted relevant
bits from the web archive replies below:

> The macro definition is not an assembly block as such, but rather just
> that: a macro definition which is supposed to be evaluated at compile
> (or assemble) time.  To reorder it on the source level seems completely
> out of the scope of LTO.

My understanding is that every "top-level" (global) invocation of the asm()
directive is converted into its own object and evaluation independently by
LTO. It doesn't matter that they're in the same file, LTO simply sees an
unordered global list of objects that do or do not reference each other.
[Googling "lto top level asm" was very informative. Seems lots of other
projects had similar issues]

The asm() block that was #include'd isn't referenced by anything so it's
dropped (which makes sense for what LTO is trying to do).

> I'm also confused because the patch is talking about link-time optimization,
> not compilation. But Pat says it causes a compilation failure. So then how
> is this related to LTO, even?

Sorry, that's a bit of sloppy language on my part. newlib with LTO compiles
just fine without this patch, but during the final link of the end application,
this becomes an issue.

> In addition, I have another question/issue. Sure, it is easy to modify the Newlib code, itself, to avoid this macro, but how can we know that there is not any user code which uses it that would now fail when it has been removed? This is a higher-level include file that users might have used; it is not a lower-level, newlib-internal file, it does not seem. That is, is it OK for compatibility reasons to just remove it (especially without a note as to why it is dangerous)?

I can't speak to that, but I can say that the current #include a macro asm
block is definitely does not work and will break with LTO, which I imagine
will become more common.

On Wed, Jan 11, 2017 at 11:50 PM, Pat Pannuto <pat.pannuto@gmail.com> 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
>         "eor    r2, r0, r1\n\t"
>         "mov    ip, r0\n\t"
>         "tst    r2, #3\n\t"
> @@ -75,7 +77,9 @@ strcpy (char* dst, const char* src)
>           load stalls.  */
>         ".p2align 2\n"
>    "2:\n\t"
> -       "optpld r1, #8\n\t"
> +#ifdef _ISA_ARM_7
> +       "pld    r1, #8\n\t"
> +#endif
>         "ldr    r4, [r1], #4\n\t"
>         "sub    r2, r3, "magic1(r5)"\n\t"
>         "bics   r2, r2, r3\n\t"
> diff --git a/newlib/libc/machine/arm/strlen-stub.c b/newlib/libc/machine/arm/strlen-stub.c
> index ea45a3789..69cfa3df0 100644
> --- a/newlib/libc/machine/arm/strlen-stub.c
> +++ b/newlib/libc/machine/arm/strlen-stub.c
> @@ -58,7 +58,9 @@ strlen (const char* str)
>         "data .req r3\n\t"
>         "addr .req r1\n\t"
>
> -       "optpld r0\n\t"
> +#ifdef _ISA_ARM_7
> +       "pld r0\n\t"
> +#endif
>         /* Word-align address */
>         "bic    addr, r0, #3\n\t"
>         /* Get adjustment for start ... */
> @@ -113,8 +115,8 @@ strlen (const char* str)
>         "ldreq  data, [addr], #4\n\t"
>         /* and 4 more bytes  */
>         "addeq  len, len, #4\n\t"
> -       /* If we have PLD, then unroll the loop a bit.  */
> -       "optpld addr, #8\n\t"
> +       /* Unroll the loop a bit.  */
> +       "pld    addr, #8\n\t"
>         /*  test (data - 0x01010101)  */
>         "ittt   eq\n\t"
>         "subeq  r2, data, ip\n\t"
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
       [not found]     ` <33404bdb-e4e3-f77e-7a97-42def0a945ff@t-online.de>
@ 2017-01-15  3:50       ` Pat Pannuto
  0 siblings, 0 replies; 13+ messages in thread
From: Pat Pannuto @ 2017-01-15  3:50 UTC (permalink / raw)
  To: Hans-Bernhard Bröker, newlib

[-- Attachment #1: Type: text/plain, Size: 5007 bytes --]

[re-send b/c list rejected large attachment; compressed and trying again]

>
>> My understanding is that every "top-level" (global) invocation of the
>> asm()
>> directive is converted into its own object and evaluation independently by
>> LTO.
>
>
> I don't think that can be the case here.  The asm() block in question is an
> (assembler) _macro_, not some piece of code that the linker would ever even
> get to see.
>
> If this construct were to fail, it would have to fail in the compiler or
> assembler.  Once past those, there's nothing left of it for the linker to do
> anything with.  The linker can't fail on something it never sees.

I'm the first to admit that I don't know exactly how LTO works, but my
understanding is that the gist of it is "compiling" in LTO-land doesn't do
very much, just saves the AST in the object to be compiled later, what's
traditionally compilation and linking are both done by the linker.

In this case, the "linker" definitely sees assembly, here's the error:

    arm-none-eabi-gcc -Wl,-pie -Wl,--gc-sections -Wl,--emit-relocs
--entry=_start -std=gnu11 -I../../libtock -DSTACK_SIZE=2048
-I../../libtock -g -mcpu=cortex-m4 -mthumb -mfloat-abi=soft -Os -flto
-ffat-lto-objects -fdata-sections -ffunction-sections -Wall -Wextra -g
-fPIC -msingle-pic-base -mpic-register=r9
-mno-pic-data-is-text-relative -T ../../linker.ld -nostdlib
-Wl,--start-group build/cortex-m4/main.o ....snip list of objects....
-Wl,--end-group -Wl,-Map=build/cortex-m4/app.Map -o
build/cortex-m4/app.elf
    <snip irrelevant warnings>
    /var/folders/_t/61hmf1293xq508hf1ksgyp900000gn/T//ccyLTEeK.s:
Assembler messages:
    /var/folders/_t/61hmf1293xq508hf1ksgyp900000gn/T//ccyLTEeK.s:821:
Error: bad instruction `return'
    lto-wrapper: fatal error: arm-none-eabi-gcc returned 1 exit status
    compilation terminated.
    /usr/local/Cellar/arm-none-eabi-gcc/6-2016-q4-update/bin/../lib/gcc/arm-none-eabi/6.2.1/../../../../arm-none-eabi/bin/ld:
error: lto-wrapper failed

I've also attached that intermediate .s file that compilation process generated,
the un-macro'd RETURN is on line 821.

>
>> The asm() block that was #include'd isn't referenced by anything so it's
>> dropped (which makes sense for what LTO is trying to do).
>
>
> It wouldn't even matter whether it's dropped or not.  By the time LTO begins
> its work, assembler macros have been applied and ceased to exist, anyway.
>
>> Sorry, that's a bit of sloppy language on my part. newlib with LTO
>> compiles
>> just fine without this patch, but during the final link of the end
>> application,
>> this becomes an issue.
>
>
> I may have overlooked it, but I don't think you've ever actually shown what
> that "issue" actually was.  So what was the actual error you observed, in
> what kind of usage?  Is there any chance for a small, self-contained test
> case that reproduces it?
>

To replicate yourself:

   - git clone https://github.com/helena-project/tock/
   - git checkout bfa44b2ed
   - cd userland/examples/rot13_client
   - make

I can try to pare down to a smaller example if needed, but it's hard to predict
when this macro thing will be an issue, seems to depend on what LTO chooses
to do

On Sat, Jan 14, 2017 at 7:10 PM, Hans-Bernhard Bröker
<HBBroeker@t-online.de> wrote:
> Am 14.01.2017 um 06:19 schrieb Pat Pannuto:
>
>>> The macro definition is not an assembly block as such, but rather just
>>> that: a macro definition which is supposed to be evaluated at compile
>>> (or assemble) time.
>
>
> Not just supposed to.  That's the only way it can be used.
>
>> My understanding is that every "top-level" (global) invocation of the
>> asm()
>> directive is converted into its own object and evaluation independently by
>> LTO.
>
>
> I don't think that can be the case here.  The asm() block in question is an
> (assembler) _macro_, not some piece of code that the linker would ever even
> get to see.
>
> If this construct were to fail, it would have to fail in the compiler or
> assembler.  Once past those, there's nothing left of it for the linker to do
> anything with.  The linker can't fail on something it never sees.
>
>> The asm() block that was #include'd isn't referenced by anything so it's
>> dropped (which makes sense for what LTO is trying to do).
>
>
> It wouldn't even matter whether it's dropped or not.  By the time LTO begins
> its work, assembler macros have been applied and ceased to exist, anyway.
>
>> Sorry, that's a bit of sloppy language on my part. newlib with LTO
>> compiles
>> just fine without this patch, but during the final link of the end
>> application,
>> this becomes an issue.
>
>
> I may have overlooked it, but I don't think you've ever actually shown what
> that "issue" actually was.  So what was the actual error you observed, in
> what kind of usage?  Is there any chance for a small, self-contained test
> case that reproduces it?
>

[-- Attachment #2: ccyLTEeK.s.bz2 --]
[-- Type: application/x-bzip2, Size: 13193 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] arm: Fix link time optimization issues
  2017-01-12  4:51 [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
                   ` (2 preceding siblings ...)
  2017-01-12  4:51 ` [PATCH 2/3] arm: Remove optpld macro Pat Pannuto
@ 2017-01-20 20:06 ` Pat Pannuto
  2017-01-25 12:32 ` Corinna Vinschen
  4 siblings, 0 replies; 13+ messages in thread
From: Pat Pannuto @ 2017-01-20 20:06 UTC (permalink / raw)
  To: newlib; +Cc: Pat Pannuto

Following up on this. Any remaining issues / anything that needs changing?

On Wed, Jan 11, 2017 at 11:50 PM, Pat Pannuto <pat.pannuto@gmail.com> wrote:
> This patch series fixes a few things so that link time optimization will work.
> The high-level problem is that LTO can re-order top-level assembly blocks, so
> code that previously worked because one asm() block was listed before another
> at global scope in a file (in this case via #include) may no longer work.
>
> So far this was only an issue with two macros that were defined in an #include'd
> header. With these patches, I have LTO working with newlib on arm.
>
> Pat Pannuto (3):
>   Remove unneeded references to arm_asm.h
>   arm: Remove optpld macro
>   arm: Remove RETURN macro
>
>  newlib/libc/machine/arm/Makefile.am           |  2 +-
>  newlib/libc/machine/arm/Makefile.in           |  2 +-
>  newlib/libc/machine/arm/aeabi_memcpy-armv7a.S |  2 +-
>  newlib/libc/machine/arm/aeabi_memmove-soft.S  |  2 --
>  newlib/libc/machine/arm/aeabi_memset-soft.S   |  2 --
>  newlib/libc/machine/arm/arm_asm.h             | 35 ---------------------------
>  newlib/libc/machine/arm/memchr.S              |  1 -
>  newlib/libc/machine/arm/strcmp-arm-tiny.S     |  2 +-
>  newlib/libc/machine/arm/strcmp-armv4.S        | 12 +++++++++
>  newlib/libc/machine/arm/strcmp-armv7m.S       |  6 ++---
>  newlib/libc/machine/arm/strcmp.S              |  1 -
>  newlib/libc/machine/arm/strcpy.c              | 20 +++++++++------
>  newlib/libc/machine/arm/strlen-armv7.S        |  2 +-
>  newlib/libc/machine/arm/strlen-stub.c         | 10 +++++---
>  14 files changed, 38 insertions(+), 61 deletions(-)
>
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] arm: Fix link time optimization issues
  2017-01-12  4:51 [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
                   ` (3 preceding siblings ...)
  2017-01-20 20:06 ` [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
@ 2017-01-25 12:32 ` Corinna Vinschen
  4 siblings, 0 replies; 13+ messages in thread
From: Corinna Vinschen @ 2017-01-25 12:32 UTC (permalink / raw)
  To: newlib

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

On Jan 11 23:50, Pat Pannuto wrote:
> This patch series fixes a few things so that link time optimization will work.
> The high-level problem is that LTO can re-order top-level assembly blocks, so
> code that previously worked because one asm() block was listed before another
> at global scope in a file (in this case via #include) may no longer work.
> 
> So far this was only an issue with two macros that were defined in an #include'd
> header. With these patches, I have LTO working with newlib on arm.
> 
> Pat Pannuto (3):
>   Remove unneeded references to arm_asm.h
>   arm: Remove optpld macro
>   arm: Remove RETURN macro
> 
>  newlib/libc/machine/arm/Makefile.am           |  2 +-
>  newlib/libc/machine/arm/Makefile.in           |  2 +-
>  newlib/libc/machine/arm/aeabi_memcpy-armv7a.S |  2 +-
>  newlib/libc/machine/arm/aeabi_memmove-soft.S  |  2 --
>  newlib/libc/machine/arm/aeabi_memset-soft.S   |  2 --
>  newlib/libc/machine/arm/arm_asm.h             | 35 ---------------------------
>  newlib/libc/machine/arm/memchr.S              |  1 -
>  newlib/libc/machine/arm/strcmp-arm-tiny.S     |  2 +-
>  newlib/libc/machine/arm/strcmp-armv4.S        | 12 +++++++++
>  newlib/libc/machine/arm/strcmp-armv7m.S       |  6 ++---
>  newlib/libc/machine/arm/strcmp.S              |  1 -
>  newlib/libc/machine/arm/strcpy.c              | 20 +++++++++------
>  newlib/libc/machine/arm/strlen-armv7.S        |  2 +-
>  newlib/libc/machine/arm/strlen-stub.c         | 10 +++++---
>  14 files changed, 38 insertions(+), 61 deletions(-)
> 
> -- 
> 2.11.0

Pushed.

Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
  2017-01-12  4:51 ` [PATCH 2/3] arm: Remove optpld macro Pat Pannuto
  2017-01-12 14:56   ` Corinna Vinschen
  2017-01-14  5:24   ` Pat Pannuto
@ 2017-01-26 10:26   ` Kyrill Tkachov
  2017-01-26 15:33     ` Corinna Vinschen
  2 siblings, 1 reply; 13+ messages in thread
From: Kyrill Tkachov @ 2017-01-26 10:26 UTC (permalink / raw)
  To: Pat Pannuto, newlib

[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]

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.


[-- Attachment #2: newlib-pld.patch --]
[-- Type: text/x-patch, Size: 1446 bytes --]

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"

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
  2017-01-26 10:26   ` Kyrill Tkachov
@ 2017-01-26 15:33     ` Corinna Vinschen
  2017-01-26 15:41       ` Kyrill Tkachov
  0 siblings, 1 reply; 13+ messages in thread
From: Corinna Vinschen @ 2017-01-26 15:33 UTC (permalink / raw)
  To: newlib; +Cc: Kyrill Tkachov

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Jan 26 10:26, Kyrill Tkachov wrote:
> 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?

Pushed.  Can you please provice patches in `git format-patch' layout in
future?  For instance, since you replied to the existing thread, the
subject in your log message needs manual intervention.

Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm: Remove optpld macro
  2017-01-26 15:33     ` Corinna Vinschen
@ 2017-01-26 15:41       ` Kyrill Tkachov
  0 siblings, 0 replies; 13+ messages in thread
From: Kyrill Tkachov @ 2017-01-26 15:41 UTC (permalink / raw)
  To: newlib


On 26/01/17 15:33, Corinna Vinschen wrote:
> On Jan 26 10:26, Kyrill Tkachov wrote:
>> 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?
> Pushed.  Can you please provice patches in `git format-patch' layout in
> future?  For instance, since you replied to the existing thread, the
> subject in your log message needs manual intervention.

Thanks, will keep that in mind.

Kyrill

> Thanks,
> Corinna
>

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-01-26 15:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12  4:51 [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
2017-01-12  4:51 ` [PATCH 1/3] Remove unneeded references to arm_asm.h Pat Pannuto
2017-01-12  4:51 ` [PATCH 3/3] arm: Remove RETURN macro Pat Pannuto
2017-01-12  4:51 ` [PATCH 2/3] arm: Remove optpld macro Pat Pannuto
2017-01-12 14:56   ` Corinna Vinschen
2017-01-12 15:56     ` Craig Howland
2017-01-14  5:24   ` Pat Pannuto
     [not found]     ` <33404bdb-e4e3-f77e-7a97-42def0a945ff@t-online.de>
2017-01-15  3:50       ` Pat Pannuto
2017-01-26 10:26   ` Kyrill Tkachov
2017-01-26 15:33     ` Corinna Vinschen
2017-01-26 15:41       ` Kyrill Tkachov
2017-01-20 20:06 ` [PATCH 0/3] arm: Fix link time optimization issues Pat Pannuto
2017-01-25 12:32 ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).