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

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

On Wednesday, February 03, 2016 10:56:06 AM Richard Earnshaw wrote:
> On 01/02/16 06:10, Thomas Preud'homme wrote:
> > 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
> 
> On my (unified tree build) I'm now seeing:
> 
> /work/rearnsha/gnu/trunk/./gcc/xgcc -B/work/rearnsha/gnu/trunk/./gcc/
> -nostdinc -B/work/rearnsha/gnu/trunk/arm-eabi/thumb/newlib/ -isystem
> /work/rearnsha/gnu/trunk/arm-eabi/thumb/newlib/targ-include -isystem
> /arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/newlib/libc/include
> -B/work/rearnsha/gnu/trunk/arm-eabi/thumb/libgloss/arm
> -L/work/rearnsha/gnu/trunk/arm-eabi/thumb/libgloss/libnosys
> -L/arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/libgloss/arm
> -B/work/rearnsha/gnu/trunk/testinstall/arm-eabi/bin/
> -B/work/rearnsha/gnu/trunk/testinstall/arm-eabi/lib/ -isystem
> /work/rearnsha/gnu/trunk/testinstall/arm-eabi/include -isystem
> /work/rearnsha/gnu/trunk/testinstall/arm-eabi/sys-include
> -L/work/rearnsha/gnu/trunk/./ld  -mthumb -DPACKAGE_NAME=\"newlib\"
> -DPACKAGE_TARNAME=\"newlib\" -DPACKAGE_VERSION=\"2.3.0\"
> -DPACKAGE_STRING=\"newlib\ 2.3.0\" -DPACKAGE_BUGREPORT=\"\"
> -DPACKAGE_URL=\"\" -I.
> -I/arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/newlib/libc/sys/arm
> -DARM_RDI_MONITOR -fno-builtin     -DARM_RDI_MONITOR -fno-builtin   -g
> -O2  -mthumb -c -o crt0.o
> /arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/newlib/libc/sys/arm/crt0.S
> In file included from
> /arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/newlib/libc/sys/arm/swi.h:1:0,
>                  from
> /arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/newlib/libc/sys/arm/libcfunc.c:
> 8:
> /arm/scratch/rearnsha/gnusrc/gcc-cross/trunk/newlib/libc/sys/arm/arm.h:32:2
> 5: fatal error: acle-compat.h: No such file or directory

My apologize, unified tree build works fine for me here. So there is no newlib 
directory (or symlink) in your build directory? Could you send me <target 
triplet>/libgloss/arm/Makefile from your build directory?

Compared to existing target doing this kind of thing the difference is that I 
use INCLUDES instead of NEWLIB_CFLAGS but as I explained, NEWLIB_CFLAGS was 
not working because it's overriden by default when doing the recursive call. 
There was the possibility of providing an ARM specific file to customize how 
to do that recursive call but that would have brought a lot of troubles.

Best regards,

Thomas