Re: Make more macro checks ARMv8-M baseline proof

On Wed, 10 Apr 2019 at 14:38, Richard Earnshaw (lists)
<[email protected]> 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 <[email protected]> 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 <[email protected]>
> >> 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