Re: [PATCH ARM/THUMB] replace mov with movs in crt0.S

[Thomas Klein <th dot r dot klein at web dot de>]

On 11/26/11 00:13, Jeff Johnston wrote:
> Richard,
>
> I'm not very keen on keeping the sys/arm stuff in newlib.  Is it 
> possible to start changing gcc to use libgloss by default?  Regarding 
> your patch, is there a corresponding change required for libgloss/arm?
>
> -- Jeff J.
>
> On 11/25/2011 10:33 AM, Richard Earnshaw wrote:
> > On 24/11/11 15:31, Richard Earnshaw wrote:
> > > And the reason I'm not seeing this is because my copy of newlib is out
> > > of date :-(
> > >
> > > Hold on...
> > >
> > > R.
> >
> > So it turns out that Thomas' patch is substantially correct, but it's
> > not all that is needed.  To get the build attributes correct we also
> > need to force the assembler to behave when building thumb1 code.  The
> > attached patch merges Thomas' changes plus some of my own to ensure that
> > this is done (and also fixes up a case where SWI was being incorrectly
> > generated for an M-class core)
> >
> > R.
> >
> >
> > <date>   Richard Earnshaw<rearnsha@arm.com>
> >     Thomas Klein<th.r.klein@web.de>
> >
> >     * sys/arm/crt0.S: Manually set the target architecture when
> >     compiling for Thumb1 on EABI targets.
> >     Don't use SWI on M-profile cores.
> >     Avoid v6-only Thumb-1 MOV instruction.

I did not completely understood the essence of the problem.
So I hope I'm not going to create too much anger.
There are two crt0.S files. One is located in newlib/libc/sys/arm and 
the other one is located libgloss/arm.
They are quite similar but not identical. Especially if dependency file 
swi.h is taking into account.
I don't know if both parts should be identical or not, nor if only one 
should exists or two.
However, they have been there for a long time.
And both are changed many times.
Now in both a little misbehavior arises.
Shouldn't this be fixed first?
If there is a bigger usability change required, shouldn't this be done 
by the maintainer itself?

Repeating previous patch for both crt0 files with minor modifications.
Removing the following line to reduce the amount of changes.

-	mov	r7, a2			/* Null frame pointer for Thumb.  */
+	movs	r7, a2			/* Null frame pointer for Thumb.  */

This code part is used either in arm mode or in thumb for cortex-m 
profile machines.
Here this makes no difference. (Due to the movs a2,#0 a few lines above).
So this change can be made or not.

The following lines are not required in libgloss

+#ifdef THUMB_V7M_V6M
+	bkpt	AngelSWI
+#else
 	AngelSWIAsm	AngelSWI
+#endif

due to different content of file swi.h.
But they should be present in newlib.

I hope that I'm not causing too much trouble.

Regards
  Thomas


ChangeLog newlib:
<date>   Richard Earnshaw<rearnsha@arm.com>
    Thomas Klein<th.r.klein@web.de>

    * libc/sys/arm/crt0.S: Manually set the target architecture
    when compiling for Thumb1 on EABI targets.
    Don't use SWI on M-profile cores.
    Avoid v6-only Thumb-1 MOV instruction.

ChangeLog libgloss:
<date>   Richard Earnshaw<rearnsha@arm.com>
    Thomas Klein<th.r.klein@web.de>

    * arm/crt0.S: Manually set the target architecture
    when compiling for Thumb1 on EABI targets.
    Avoid v6-only Thumb-1 MOV instruction.

Attachment: diff.newlib
Description: Text document

Attachment: diff.libgloss
Description: Text document