Re: [Patch, microblaze]: Add a small Linux BSP

[Michael Eager <eager at eagerm dot com>]

On 07/26/13 07:54, David Holsgrove wrote:
> > -----Original Message-----
> > From: Michael Eager [mailto:eager@eagerm.com]
> > Sent: Friday, 26 July 2013 1:17 pm
> > To: Edgar Iglesias
> > Cc: David Holsgrove; newlib@sourceware.org; vinschen@redhat.com; Edgar
> > Iglesias
> > Subject: Re: [Patch, microblaze]: Add a small Linux BSP
> >
> > On 07/25/13 19:43, Edgar E. Iglesias wrote:
> > > On Fri, Jul 26, 2013 at 04:10:51AM +0200, David Holsgrove wrote:
> > > > > -----Original Message-----
> > > > > From: Michael Eager [mailto:eager@eagerm.com]
> > > > > Sent: Friday, 26 July 2013 11:43 am
> > > > > To: newlib@sourceware.org; vinschen@redhat.com; David Holsgrove
> > > > > Subject: Re: [Patch, microblaze]: Add a small Linux BSP
> > > > >
> > > > > On 07/23/13 02:11, Corinna Vinschen wrote:
> > > > > > On Jul 23 17:16, David Holsgrove wrote:
> > > > > > > On 23 July 2013 17:10, Corinna Vinschen <vinschen@redhat.com> wrote:
> > > > > > > > On Jul 23 16:25, David Holsgrove wrote:
> > > > > > > > > From: "David Holsgrove" <...>
> > > > > > > > >
> > > > > > > > > libgloss/ChangeLog
> > > > > > > > >
> > > > > > > > > 2013-07-23  Edgar E. Iglesias <...>
> > > > > > > > >
> > > > > > > > >      * microblaze/Makefile.in: Add LINUX_BSP.
> > > > > > > > >      * microblaze/elf-gloss-linux.ld: New file.
> > > > > > > > >      * microblaze/elf-gloss-linux.specs: Likewise.
> > > > > > > > >      * microblaze/linux-crt0.S: Likewise.
> > > > > > > > >      * microblaze/linux-inbyte.c: Likewise.
> > > > > > > > >      * microblaze/linux-outbyte.c: Likewise.
> > > > > > > > >      * microblaze/linux-syscalls-wrap.c: Likewise.
> > > > > > > > >      * microblaze/linux-syscalls.S: Likewise.
> > > > > > > > >      * microblaze/linux-syscalls.h: Likewise.
> > > > > > > >
> > > > > > > > Please apply.
> > > > > > >
> > > > > > > Thanks Corinna,
> > > > > > >
> > > > > > > I don't have write perms on newlib - can I request them, or can you
> > > > > > > apply the patch for me?
> > > > > >
> > > > > > Everything applied.
> > > > >
> > > > > This patch breaks breaks existing mb-gcc compilations.
> > > > >
> > > > > Please revert the patch until it can be fixed.
> > > > Hi Michael,
> > > >
> > > > Can you provide details regarding the breakage?
> > > >
> > > > This should be a separate library and not linked in by default - what sort
> > > > of compilation errors are you seeing?
> > >
> > >
> > > libgloss/sbrk.o is being linked into executables instead of
> > > libgloss/microblaze/sbrk.o, which results in an unresolved reference to _end.
> >
> > Here is a sample test:
> > mb-gcc testsuite/gcc.c-torture/execute/strcpy-1.c  -fno-diagnostics-show-caret
> > -fdiagnostics-color=never  -w  -O3 -fomit-frame-pointer  -mno-xl-soft-mul
> > -mxl-barrel-shift -mcpu=v6.00.a -Wl,-defsym -Wl,_HEAP_SIZE=0x1000000 -Wl,-
> > defsym
> > -Wl,_STACK_SIZE=0x80000 -L microblaze_0/lib -lxil
> > -TLinkScr.ld  -lm   -o strcpy-1.x3
> >
> > LinkScr.ld and libxil are from either EDK9.2i or EDK-14.2.
> >
> > I don't have the failure log at the moment.
>
> Thanks Michael for the details.
>
> I've managed to recreate the unresolved reference to _end you see with your
> testcase, but only by using the linker script you provided to me in the past from
> EDK9.2i - when using your LinkScr.ld from EDK-14.2 I don't see any undefs.

Thanks for investigating this.

> The reason for this is that the linker script generated by libgen in XSDK 14.2 (and since
> at least XSDK version 13.1 it would appear) includes;
>
> _end = .;
>
> This is why our testing succeed where yours didnât - we were using more modern
> versions of XSDK to provide libxil and the linker script - but this is actually just
> masking a configuration error.

Correct.

> The patch added in http://sourceware.org/ml/newlib/2013/msg00551.html utilised
> the existing libgloss target_makefile_frag to define generic objects to be linked
> into libgloss.a - something which has long been missing for microblaze in FSF newlib.
>
> What I didnât see was that even though we made no changes to the sbrk.c compile
> stage, the inclusion of libgloss/config/default.mt overrides where sbrk.c came from,
> using the generic libgloss/sbrk.c instead of the libgloss/microblaze/sbrk.c copy.
>
> The differences between these two are quite small - and perhaps should be addressed
> in a future patch to reduce the amount of microblaze specific variants for you to maintain,
> but most importantly _end is used in the generic instead of _heap in the microblaze copy;
>
>    if (!heap_ptr)
>      heap_ptr = (char *)&_end;
>    base = heap_ptr;
>    heap_ptr += nbytes;
>
>    if (!heap_ptr)
>      heap_ptr = (char *)&_heap;
>    base = heap_ptr;
>    heap_ptr += nbytes;

Yes, I don't see a good reason that there are microblaze-specific routines which
provide the same functionality as the generic functions.

> So in summary, the libgloss-linux.a added by the patch in this thread does not cause any
> regressions.

I didn't think that a Linux-specific library which was not included in my tests
caused a regression.

> The patch added in http://sourceware.org/ml/newlib/2013/msg00551.html should
> be amended to ensure microblaze/sbrk.c is used for the moment - I'll provide a patch
> to do so now.

Thanks.

> We should also establish a common test framework for microblaze targets to avoid
> difference such as these in future which can be triggered by varying versions of Xilinx
> provided tools.

I've suggested this for quite a few months.

> I donât suppose your dejagnu boards are available through GNU's dejagnu project?
> This can be discussed off list, as I believe this thread has already gotten off the topic
> of adding this LINUX_BSP.

The MB test configuration is not in the DejaGNU project.


--
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077