Re: [PATCH, ping1] Make macro checks ARMv8-M baseline proof

[Thomas Preud'homme <thomas dot preudhomme at foss dot arm dot com>]

On Thursday, January 28, 2016 11:28:11 AM Corinna Vinschen wrote:
> On Jan 21 11:32, Thomas Preud'homme wrote:
> > Ping?
> > 
> > On Friday, January 15, 2016 05:52:40 PM Thomas Preud'homme wrote:
> > > On Wednesday, January 13, 2016 10:41:22 AM Thomas Preud'homme wrote:
> > > > Hi Richard,
> > > > 
> > > > NEWLIB_CFLAGS, CFLAGS and CFLAGS_FOR_TARGET in libgloss/arm/Makefile
> > > > are
> > > > all overriden when libgloss/Makefile does the recursive call (see
> > > > FLAGS_TO_PASS). So instead I extended INCLUDES to contains the newlib
> > > > directory containing acle-compat.h. Note that nothing is done when
> > > > libgloss
> > > > is built alone but this means old GCC cannot build libgloss alone.
> > > > 
> > > > Note that, as I explained in [1], there is two code change with this
> > > > patch
> > > > but I believe they are actually bug fixes. Please let me know if you
> > > > agree
> > > > with my analysis.
> > > > 
> > > > [1] https://sourceware.org/ml/newlib/2015/msg00955.html
> > > 
> > > Except that I was (doubly) wrong for one of them. In that message, I
> > > wrote
> > > 
> > > > This change include armv7 in PREFER_THUMB where it was not included in
> > > > THUMB_V7_V6M. It leads to use of svc to bkpt and the mov.w fp, #0 in
> > > > redboot-crt0.S is no longer included.
> > > 
> > > First of all, the issue is not in PREFER_THUMB but in replacing
> > > THUMB_V6M_V7M by !__ARM_ARCH_ISA_ARM. Secondly, this is indeed a bug
> > > because the bkpt/svc in question is for semihosting and only M profile
> > > architectures should use bkpt according to the semihosting interface
> > > [1].
> > > The mov.w fp, #0 on the other hand is a correct change since there is no
> > > ARM state in ARMv7 (being the common subset of ARMv7-M, ARMv7-R and
> > > ARMv7-A).
> > > 
> > > [1]
> > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471l/
> > > pge1358787048379.html
> > > 
> > > I've thus respinned the patch, keeping THUMB_V6M_V7M (but under the new
> > > name THUMB_VxM) everywhere except for fp enabling.
> > > 
> > > ChangeLog entries are now as follow:
> > > 
> > > 
> > > *** libgloss/ChangeLog ***
> > > 
> > > 2015-01-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > > 
> > >         * arm/Makefile.in: Add newlib/libc/machine/arm to the include
> > >         path
> > > 
> > > if newlib is present.
> > > 
> > >         * arm/arm.h: Include acle-compat.h.
> > >         (THUMB_V7_V6M): Rename to ...
> > >         (PREFER_THUMB): This.  Use ACLE macros __ARM_ARCH_ISA_ARM
> > >         instead of
> > > 
> > > __ARM_ARCH_6M__ to decide whether to define it.
> > > 
> > >         (THUMB1_ONLY): Define for Thumb-1 only targets.
> > >         (THUMB_V7M_V6M): Rename to ...
> > >         (THUMB_VXM): This.  Defined based on __ARM_ARCH_ISA_ARM,
> > >         excluding
> > >         ARMv7.
> > >         * arm/crt0.S: Use THUMB1_ONLY rather than __ARM_ARCH_6M__,
> > >         !__ARM_ARCH_ISA_ARM rather than THUMB_V7M_V6M for fp enabling,
> > >         and
> > >         PREFER_THUMB rather than THUMB_V7_V6M.  Rename other occurences
> > >         of
> > >         THUMB_V7M_V6M to THUMB_VXM.
> > >         * arm/linux-crt0.c: Likewise.
> > >         * arm/redboot-crt0.S: Likewise.
> > >         * arm/swi.h: Likewise.
> > >         * arm/trap.S: Likewise.
> > > 
> > > *** newlib/ChangeLog ***
> > > 
> > > 2015-01-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > > 
> > >         * libc/machine/arm/memcpy-stub.c: Use ACLE macros
> > > 
> > > __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to check for Thumb-2 only
> > > targets rather than __ARM_ARCH and __ARM_ARCH_PROFILE.
> > > 
> > >         * libc/machine/arm/memcpy.S: Likewise.
> > >         * libc/machine/arm/setjmp.S: Likewise for Thumb-1 only target
> > >         and
> > >         include acle-compat.h.
> > >         * libc/machine/arm/strcmp.S: Likewise for Thumb-1 and Thumb-2
> > >         only
> > >         target and include acle-compat.h.
> > >         * libc/sys/arm/arm.h: Include acle-compat.h.
> > >         (THUMB_V7_V6M): Rename to ...
> > >         (PREFER_THUMB): This.  Use ACLE macro __ARM_ARCH_ISA_ARM instead
> > >         of
> > >         __ARM_ARCH_6M__ to decide whether to define it.
> > >         (THUMB1_ONLY): Define for Thumb-1 only targets.
> > >         (THUMB_V7M_V6M): Rename to ...
> > >         (THUMB_VXM): This.  Defined based on __ARM_ARCH_ISA_ARM,
> > >         excluding
> > >         ARMv7.
> > >         * libc/sys/arm/crt0.S: Use PREFER_THUMB rather than THUMB_V7_V6M
> > >         and
> > > 
> > > rename THUMB_V7M_V6M into THUMB_VXM.
> > > 
> > >         * libc/sys/arm/swi.h: Likewise.
> > > 
> > > [...]
> 
> Applied.

Thanks Corinna.

Best regards,

Thomas