* [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants @ 2019-09-30 15:16 Christos Gentsos 2019-10-07 14:42 ` Richard Earnshaw (lists) 0 siblings, 1 reply; 4+ messages in thread From: Christos Gentsos @ 2019-09-30 15:16 UTC (permalink / raw) To: newlib In the ARMv6-M Thumb instruction set the MOV, ADD and SUB instructions don't support flag-preserving variants for their 8-bit immediate forms. --- newlib/libc/machine/arm/setjmp.S | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/newlib/libc/machine/arm/setjmp.S b/newlib/libc/machine/arm/setjmp.S index 1ba711d5..b8511779 100644 --- a/newlib/libc/machine/arm/setjmp.S +++ b/newlib/libc/machine/arm/setjmp.S @@ -74,11 +74,11 @@ SYM (setjmp): mov r5, sp mov r6, lr stmia r0!, {r1, r2, r3, r4, r5, r6} - sub r0, r0, #40 + subs r0, r0, #40 /* Restore callee-saved low regs. */ ldmia r0!, {r4, r5, r6, r7} /* Return zero. */ - mov r0, #0 + movs r0, #0 bx lr .thumb_func @@ -86,7 +86,7 @@ SYM (setjmp): TYPE (longjmp) SYM (longjmp): /* Restore High regs. */ - add r0, r0, #16 + adds r0, r0, #16 ldmia r0!, {r2, r3, r4, r5, r6} mov r8, r2 mov r9, r3 @@ -95,12 +95,12 @@ SYM (longjmp): mov sp, r6 ldmia r0!, {r3} /* lr */ /* Restore low regs. */ - sub r0, r0, #40 + subs r0, r0, #40 ldmia r0!, {r4, r5, r6, r7} /* Return the result argument, or 1 if it is zero. */ mov r0, r1 bne 1f - mov r0, #1 + movs r0, #1 1: bx r3 -- 2.23.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants 2019-09-30 15:16 [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants Christos Gentsos @ 2019-10-07 14:42 ` Richard Earnshaw (lists) 2019-10-09 13:28 ` Christos Gentsos 0 siblings, 1 reply; 4+ messages in thread From: Richard Earnshaw (lists) @ 2019-10-07 14:42 UTC (permalink / raw) To: Christos Gentsos, newlib On 30/09/2019 16:15, Christos Gentsos wrote: > In the ARMv6-M Thumb instruction set the MOV, ADD and SUB instructions > don't support flag-preserving variants for their 8-bit immediate > forms. > --- > newlib/libc/machine/arm/setjmp.S | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/newlib/libc/machine/arm/setjmp.S b/newlib/libc/machine/arm/setjmp.S > index 1ba711d5..b8511779 100644 > --- a/newlib/libc/machine/arm/setjmp.S > +++ b/newlib/libc/machine/arm/setjmp.S > @@ -74,11 +74,11 @@ SYM (setjmp): > mov r5, sp > mov r6, lr > stmia r0!, {r1, r2, r3, r4, r5, r6} > - sub r0, r0, #40 > + subs r0, r0, #40 > /* Restore callee-saved low regs. */ > ldmia r0!, {r4, r5, r6, r7} > /* Return zero. */ > - mov r0, #0 > + movs r0, #0 > bx lr > > .thumb_func > @@ -86,7 +86,7 @@ SYM (setjmp): > TYPE (longjmp) > SYM (longjmp): > /* Restore High regs. */ > - add r0, r0, #16 > + adds r0, r0, #16 > ldmia r0!, {r2, r3, r4, r5, r6} > mov r8, r2 > mov r9, r3 > @@ -95,12 +95,12 @@ SYM (longjmp): > mov sp, r6 > ldmia r0!, {r3} /* lr */ > /* Restore low regs. */ > - sub r0, r0, #40 > + subs r0, r0, #40 > ldmia r0!, {r4, r5, r6, r7} > /* Return the result argument, or 1 if it is zero. */ > mov r0, r1 > bne 1f > - mov r0, #1 > + movs r0, #1 > 1: > bx r3 > > Sorry for the delay responding to this one, I wanted to look at this in more detail... Having done so, I don't think this is correct. The code you are looking at here is written in legacy syntax, rather than unified syntax. In legacy syntax there was no need to specify the flag-clobbering behaviour of instructions. For example, the longjump code does not make sense otherwise as there is no explicit comparison operation, yet there is a conditional branch near the end: > sub r0, r0, #40 > ldmia r0!, {r4, r5, r6, r7} > /* Return the result argument, or 1 if it is zero. */ > mov r0, r1 // <<<< implicit flag set .... > bne 1f // <<<< ... only makes sense in legacy syntax > mov r0, #1 > 1: How did you build and test this? R. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants 2019-10-07 14:42 ` Richard Earnshaw (lists) @ 2019-10-09 13:28 ` Christos Gentsos 2019-10-09 13:35 ` Emmanuel Blot 0 siblings, 1 reply; 4+ messages in thread From: Christos Gentsos @ 2019-10-09 13:28 UTC (permalink / raw) To: Richard Earnshaw (lists), newlib Oh I see, you're totally right. I was trying to use Clang to compile newlib, I wasn't aware that its assembler didn't fully support the legacy ARM syntax - it was trying to parse it as unified syntax code. Using GCC it compiles just fine. The longjmp example you brought up also makes perfect sense, it seems that the code I used my newlib build with only worked because it wasn't using it. Thanks so much for your detailed reply and for the time you took to look into this! Cheers, Christos On Mon, Oct 07 2019 at 15:42:21 +0100, Richard Earnshaw (lists) wrote: > On 30/09/2019 16:15, Christos Gentsos wrote: >> In the ARMv6-M Thumb instruction set the MOV, ADD and SUB instructions >> don't support flag-preserving variants for their 8-bit immediate >> forms. >> --- >> newlib/libc/machine/arm/setjmp.S | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/newlib/libc/machine/arm/setjmp.S b/newlib/libc/machine/arm/setjmp.S >> index 1ba711d5..b8511779 100644 >> --- a/newlib/libc/machine/arm/setjmp.S >> +++ b/newlib/libc/machine/arm/setjmp.S >> @@ -74,11 +74,11 @@ SYM (setjmp): >> mov r5, sp >> mov r6, lr >> stmia r0!, {r1, r2, r3, r4, r5, r6} >> - sub r0, r0, #40 >> + subs r0, r0, #40 >> /* Restore callee-saved low regs. */ >> ldmia r0!, {r4, r5, r6, r7} >> /* Return zero. */ >> - mov r0, #0 >> + movs r0, #0 >> bx lr >> >> .thumb_func >> @@ -86,7 +86,7 @@ SYM (setjmp): >> TYPE (longjmp) >> SYM (longjmp): >> /* Restore High regs. */ >> - add r0, r0, #16 >> + adds r0, r0, #16 >> ldmia r0!, {r2, r3, r4, r5, r6} >> mov r8, r2 >> mov r9, r3 >> @@ -95,12 +95,12 @@ SYM (longjmp): >> mov sp, r6 >> ldmia r0!, {r3} /* lr */ >> /* Restore low regs. */ >> - sub r0, r0, #40 >> + subs r0, r0, #40 >> ldmia r0!, {r4, r5, r6, r7} >> /* Return the result argument, or 1 if it is zero. */ >> mov r0, r1 >> bne 1f >> - mov r0, #1 >> + movs r0, #1 >> 1: >> bx r3 >> >> > > Sorry for the delay responding to this one, I wanted to look at this in > more detail... > > Having done so, I don't think this is correct. The code you are looking > at here is written in legacy syntax, rather than unified syntax. In > legacy syntax there was no need to specify the flag-clobbering behaviour > of instructions. > > For example, the longjump code does not make sense otherwise as there is > no explicit comparison operation, yet there is a conditional branch near > the end: > > > sub r0, r0, #40 > > ldmia r0!, {r4, r5, r6, r7} > > /* Return the result argument, or 1 if it is zero. */ > > mov r0, r1 // <<<< implicit flag set .... > > bne 1f // <<<< ... only makes sense in legacy syntax > > mov r0, #1 > > 1: > > How did you build and test this? > > R. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants 2019-10-09 13:28 ` Christos Gentsos @ 2019-10-09 13:35 ` Emmanuel Blot 0 siblings, 0 replies; 4+ messages in thread From: Emmanuel Blot @ 2019-10-09 13:35 UTC (permalink / raw) To: Christos Gentsos; +Cc: Richard Earnshaw, newlib Hi Christos, On 9 Oct 2019, at 15:27, Christos Gentsos wrote: > Oh I see, you're totally right. > > I was trying to use Clang to compile newlib, I wasn't aware that its > assembler didn't fully support the legacy ARM syntax - it was trying > to > parse it as unified syntax code. Using GCC it compiles just fine. FWIW, there are a few issues to build newlib with clang for ARMv6-M and ARMv7-(E)M, but I’ve been building and using it with several clang versions (with the help of a couple of patches) with no noticeable issues on baremetal targets. Emmanuel. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-09 13:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-30 15:16 [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants Christos Gentsos 2019-10-07 14:42 ` Richard Earnshaw (lists) 2019-10-09 13:28 ` Christos Gentsos 2019-10-09 13:35 ` Emmanuel Blot
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).