Make more macro checks ARMv8-M baseline proof
Christophe Lyon
christophe.lyon@linaro.org
Thu Apr 11 14:21:00 GMT 2019
On Thu, 11 Apr 2019 at 15:35, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>
> On 10/04/2019 16:12, Christophe Lyon wrote:
> > On Wed, 10 Apr 2019 at 15:22, Richard Earnshaw (lists)
> > <Richard.Earnshaw@arm.com> wrote:
> >>
> >> On 10/04/2019 13:52, Christophe Lyon wrote:
> >>> On Wed, 10 Apr 2019 at 14:38, Richard Earnshaw (lists)
> >>> <Richard.Earnshaw@arm.com> wrote:
> >>>>
> >>>> On 01/04/2019 18:33, Christophe Lyon wrote:
> >>>>> I forgot that the suitable format in this project is via git
> >>>>> format-patch, so here is the same patch in that format.
> >>>>>
> >>>>> Christophe
> >>>>>
> >>>>> On Mon, 1 Apr 2019 at 16:42, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> After Thomas' patch
> >>>>>> (https://sourceware.org/ml/newlib/2016/msg00017.html), it seems that
> >>>>>> some changes committed to libgloss are missing in newlib.
> >>>>>> I noticed the problem when building a toolchain where GCC is
> >>>>>> configured --with-cpu=cortex-m{0,23}.
> >>>>>>
> >>>>>> This small patch brings the necessary changes from libgloss to newlib.
> >>>>>> With this, the GCC toolchain build completes.
> >>>>>>
> >>>>>> OK?
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>> Christophe
> >>>>>>
> >>>>>> 0001-Make-more-macro-checks-ARMv8-M-baseline-proof.patch
> >>>>>>
> >>>>>> From 1ea9c597edbece03bce084afe33b45f1ee9ff67f Mon Sep 17 00:00:00 2001
> >>>>>> From: Christophe Lyon <christophe.lyon@linaro.org>
> >>>>>> Date: Mon, 1 Apr 2019 17:30:53 +0000
> >>>>>> Subject: [PATCH 1/1] Make more macro checks ARMv8-M baseline proof.
> >>>>>>
> >>>>>> Commit 69f4c4029183fb26d2fcae00790881620c1978a3 improved most
> >>>>>> macro checks to be ARMv8-M baseline proof, but missed a few
> >>>>>> occurrences which otherwise fail to build when using a CPU setting
> >>>>>> such as cortex-m0 or cortex-m23. This patch brings the same
> >>>>>> changes as the ones that were committed to libgloss at that time.
> >>>>>>
> >>>>>> newlib:
> >>>>>> * libc/sys/arm/crt0.S: Use THUMB1_ONLY rather than
> >>>>>> __ARM_ARCH_6M__.
> >>>>>> * libc/sys/arm/trap.S: Use PREFER_THUMB rather than __thumb2__.
> >>>>>> ---
> >>>>>> newlib/libc/sys/arm/crt0.S | 8 ++++----
> >>>>>> newlib/libc/sys/arm/trap.S | 3 ++-
> >>>>>> 2 files changed, 6 insertions(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S
> >>>>>> index 64d4259..8c9f7be 100644
> >>>>>> --- a/newlib/libc/sys/arm/crt0.S
> >>>>>> +++ b/newlib/libc/sys/arm/crt0.S
> >>>>>> @@ -85,7 +85,7 @@
> >>>>>>
> >>>>>> /* Stack limit is at end of data. */
> >>>>>> /* Allow slop for stack overflow handling and small frames. */
> >>>>>> -#ifdef __ARM_ARCH_6M__
> >>>>>> +#ifdef THUMB1_ONLY
> >>>>>> ldr r0, .LC2
> >>>>>> adds r0, #128
> >>>>>> adds r0, #128
> >>>>>> @@ -137,7 +137,7 @@
> >>>>>> beq .LC27
> >>>>>>
> >>>>>> /* Allow slop for stack overflow handling and small frames. */
> >>>>>> -#ifdef __ARM_ARCH_6M__
> >>>>>> +#ifdef THUMB1_ONLY
> >>>>>> adds r2, #128
> >>>>>> adds r2, #128
> >>>>>> mov sl, r2
> >>>>>> @@ -164,7 +164,7 @@
> >>>>>> #ifdef __thumb2__
> >>>>>> it eq
> >>>>>> #endif
> >>>>>> -#ifdef __ARM_ARCH_6M__
> >>>>>> +#ifdef THUMB1_ONLY
> >>>>>> bne .LC28
> >>>>>> ldr r3, .LC0
> >>>>>> .LC28:
> >>>>>> @@ -219,7 +219,7 @@
> >>>>>> this default 64k is enough for the program being executed.
> >>>>>> However, it ensures that this simple crt0 world will not
> >>>>>> immediately cause an overflow event: */
> >>>>>> -#ifdef __ARM_ARCH_6M__
> >>>>>> +#ifdef THUMB1_ONLY
> >>>>>> movs r2, #64
> >>>>>> lsls r2, r2, #10
> >>>>>> subs r2, r3, r2
> >>>>
> >>>> The above are all OK.
> >>>>
> >>>>>> diff --git a/newlib/libc/sys/arm/trap.S b/newlib/libc/sys/arm/trap.S
> >>>>>> index 21b6937..d854b57 100644
> >>>>>> --- a/newlib/libc/sys/arm/trap.S
> >>>>>> +++ b/newlib/libc/sys/arm/trap.S
> >>>>>> @@ -1,5 +1,6 @@
> >>>>>> +#include "arm.h"
> >>>>>> /* Run-time exception support */
> >>>>>> -#if !defined(__thumb2__)
> >>>>>> +#ifndef PREFER_THUMB
> >>>>>> #include "swi.h"
> >>>>>>
> >>>>>> /* .text is used instead of .section .text so it works with arm-aout too. */
> >>>>
> >>>> I'm not sure I understand this hunk. PREFER_THUMB is no the same as
> >>>> __thumb2__, so I think this needs more clarification. Why would it be
> >>>> right to omit the contents of this file entirely? what provides this
> >>>> API if this doesn't?
> >>>>
> >>>> I'm not sure I like the PREFER_THUMB macro anyway: preference is not the
> >>>> same as 'must be' or 'can only be'; so this is all somewhat confusing.
> >>>
> >>> Well, this is exactly the same change as the one applied by commit
> >>> 69f4c4029183fb26d2fcae00790881620c1978a3 to libgloss/arm/trap.S.
> >>>
> >>> Do you mean both trap.S could/should be different, or that commit
> >>> 69f4c4029183fb26d2fcae00790881620c1978a3 is wrong?
> >>>
> >>> Christophe
> >>>
> >>
> >> I didn't review that patch :-(
> >>
> >> The effect of Thomas' patch was to remove this API on systems that built
> >> newlib in Thumb1 mode for use on Armv4t through Armv6 (ie on systems
> >> with both Thumb1 and ARM ISAs); that was probably unintentional.
> >>
> >> But in fact, I think even the original patch from Paul Brook was
> >> probably not correct as the effect was to remove this API on A-profile
> >> targets based solely on which ISA was selected for building the library.
> >>
> >> Does this all matter in reality? Probably not. Looking in more detail
> >> I see the code in this function is to support the old APCS chunked stack
> >> variant, which dates back to the Acorn days; GCC hasn't really supported
> >> that since forever, and it certainly won't work in EABI environments.
> >>
> >> So a better patch would be to change this to
> >>
> >> #ifndef __ARM_EABI__
> >>
> >> and to make the same change in libgloss.
> >>
> >
> > Thanks for the clarification, I wouldn't have inferred the APCS part myself.
> >
> > Here are two patches: the 1st one contains the part you already approved,
> > the 2nd one contains the __ARM_EABI__ change for both newlib and libgloss.
> >
> > (I split the change into two parts because now they are not strictly related)
> >
> > OK?
> >
>
> Yes, both changes are fine.
>
> R.
Thanks, pushed.
>
> > Thanks,
> >
> > Christophe
> >
> >> R.
> >>
> >> 0002-arm-Include-code-in-trap.S-for-APCS-only.patch
> >>
> >> From 390afa88e3d03ce4838f377978a9d7f2c9d468a7 Mon Sep 17 00:00:00 2001
> >> From: Christophe Lyon <christophe.lyon@linaro.org>
> >> Date: Wed, 10 Apr 2019 15:04:13 +0000
> >> Subject: [PATCH 2/2] [arm] Include code in trap.S for APCS only.
> >>
> >> The code in trap.S is to support the old APCS chunked stack variant,
> >> which dates back to the Acorn days, so put it under #ifndef
> >> __ARM_EABI__.
> >>
> >> * libgloss/arm/trap.S: Use __ARM_EABI rather than PREFER_THUMB.
> >> * newlib/libc/sys/arm/trap.S: Use __ARM_EABI rather than
> >> __thumb2__.
> >> ---
> >> libgloss/arm/trap.S | 2 +-
> >> newlib/libc/sys/arm/trap.S | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libgloss/arm/trap.S b/libgloss/arm/trap.S
> >> index d854b57..845ad01 100644
> >> --- a/libgloss/arm/trap.S
> >> +++ b/libgloss/arm/trap.S
> >> @@ -1,6 +1,6 @@
> >> #include "arm.h"
> >> /* Run-time exception support */
> >> -#ifndef PREFER_THUMB
> >> +#ifndef __ARM_EABI__
> >> #include "swi.h"
> >>
> >> /* .text is used instead of .section .text so it works with arm-aout too. */
> >> diff --git a/newlib/libc/sys/arm/trap.S b/newlib/libc/sys/arm/trap.S
> >> index 21b6937..681b3db 100644
> >> --- a/newlib/libc/sys/arm/trap.S
> >> +++ b/newlib/libc/sys/arm/trap.S
> >> @@ -1,5 +1,5 @@
> >> /* Run-time exception support */
> >> -#if !defined(__thumb2__)
> >> +#ifndef __ARM_EABI__
> >> #include "swi.h"
> >>
> >> /* .text is used instead of .section .text so it works with arm-aout too. */
> >>
> >> 0001-Make-more-macro-checks-ARMv8-M-baseline-proof.patch
> >>
> >> From 2c57cf3aa28614ba2529c1f45bc45fb971121535 Mon Sep 17 00:00:00 2001
> >> From: Christophe Lyon <christophe.lyon@linaro.org>
> >> Date: Mon, 1 Apr 2019 17:30:53 +0000
> >> Subject: [PATCH 1/2] Make more macro checks ARMv8-M baseline proof.
> >>
> >> Commit 69f4c4029183fb26d2fcae00790881620c1978a3 improved most
> >> macro checks to be ARMv8-M baseline proof, but missed a few
> >> occurrences which otherwise fail to build when using a CPU setting
> >> such as cortex-m0 or cortex-m23. This patch brings the same
> >> changes as the ones that were committed to libgloss at that time.
> >>
> >> newlib:
> >> * libc/sys/arm/crt0.S: Use THUMB1_ONLY rather than
> >> __ARM_ARCH_6M__.
> >> ---
> >> newlib/libc/sys/arm/crt0.S | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S
> >> index 64d4259..8c9f7be 100644
> >> --- a/newlib/libc/sys/arm/crt0.S
> >> +++ b/newlib/libc/sys/arm/crt0.S
> >> @@ -85,7 +85,7 @@
> >>
> >> /* Stack limit is at end of data. */
> >> /* Allow slop for stack overflow handling and small frames. */
> >> -#ifdef __ARM_ARCH_6M__
> >> +#ifdef THUMB1_ONLY
> >> ldr r0, .LC2
> >> adds r0, #128
> >> adds r0, #128
> >> @@ -137,7 +137,7 @@
> >> beq .LC27
> >>
> >> /* Allow slop for stack overflow handling and small frames. */
> >> -#ifdef __ARM_ARCH_6M__
> >> +#ifdef THUMB1_ONLY
> >> adds r2, #128
> >> adds r2, #128
> >> mov sl, r2
> >> @@ -164,7 +164,7 @@
> >> #ifdef __thumb2__
> >> it eq
> >> #endif
> >> -#ifdef __ARM_ARCH_6M__
> >> +#ifdef THUMB1_ONLY
> >> bne .LC28
> >> ldr r3, .LC0
> >> .LC28:
> >> @@ -219,7 +219,7 @@
> >> this default 64k is enough for the program being executed.
> >> However, it ensures that this simple crt0 world will not
> >> immediately cause an overflow event: */
> >> -#ifdef __ARM_ARCH_6M__
> >> +#ifdef THUMB1_ONLY
> >> movs r2, #64
> >> lsls r2, r2, #10
> >> subs r2, r3, r2
>
>
More information about the Newlib
mailing list