Re: [Tinycc-devel] Regression on ARM

[Kirill Smelkov]

On Wed, Nov 21, 2012 at 12:30:04PM +0100, Thomas Preud'homme wrote:
> On Wed, 21 Nov 2012 10:03:58 +0400, Kirill Smelkov wrote:
> >On Tue, Nov 20, 2012 at 03:29:26PM +0100, address@hidden wrote:
> >>Another regression is due to the unconditional activation of btest
> >>in the list of tests to run. Indeed, there is no bound checking on
> >>arm (and also C67 and il). This test should thus be run only for
> >>architecture supporting it.
> >
> >Sorry, my fault, that was late at night. Does the following patch
> >fixes
> >btest issue for you?
> >
> >---- 8< ----
> >From: Kirill Smelkov <address@hidden>
> >Date: Wed, 21 Nov 2012 09:47:23 +0400
> >Subject: [PATCH] tests: btest should only run on targets
> >supporting bcheck
> >
> >After 40a54c43 (Repair bounds-checking runtime), and in particular
> >5d648485 (Now btest pass!) `make test` was broken on ARCH != i386,
> >because I've changed btest to unconditionally run on all arches.
> >
> >But bounds-checking itself is only supported on i386 and oops...
> >
> >My fault, fix it.
> 
> No need to blame yourself, this line is not necessary. Besides, this
> information can be extracted from git log/blame.

I don't insist, let's drop that line.


> >Reported-by: Thomas Preud'homme <address@hidden>
> >---
> > tests/Makefile | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/tests/Makefile b/tests/Makefile
> >index af1fdb8..ca581b0 100644
> >--- a/tests/Makefile
> >+++ b/tests/Makefile
> >@@ -11,12 +11,16 @@ TESTS = libtest \
> >             test2 \
> >             test3 \
> >             speedtest \
> >-            btest \
> >             weaktest
> >
> >             # test4 # this test does not seem to work on any platform
> >             # asmtest # this test does not seem to work on any platform
> >
> >+# bounds-checking is supported only on i386
> >+ifeq ($(ARCH),i386)
> >+    TESTS += btest
> >+endif
> >+
> > # these should work too
> > # TESTS += test1 test2 speedtest btest weaktest
> 
> >
> >--
> >1.8.0.289.g7a667bc
> 
> Yep it works but shouldn't you use the same filter-out method to be
> consistent with other disabled tests?

I don't like that filter-out's. Actually, what I'd like to write here
is

    ifeq ($(CONFIG_TCC_BCHECK),y)
        TESTS += btest
    endif

or even better

    TESTS-$(CONFIG_TCC_BCHECK) += btest

and that CONFIG_TCC_BCHECK be inferred from cpu, arch, os, etc... by
configure to "y" or "n" and only TESTS-y get used.

This way that CONFIG_TCC_BCHECK setting logic would be in only one
place, contrast to lots if big ifdefs/filter-outs everywhere.

This config scheme is used by linux kernel.


Only I don't have enough free time, that's why I don't insist. The
ifeq($(ARCH),i386) was closer to what I would like it to be - that's why
I originally used it.

Again, I don't insist.


> >>There was a test regression on ARM recently which I managed to
> >>pinpoint to commit b2a02961b4407ca76db5a66ca5ae855cbfba4e8e. All I
> >>can tell so far is that it's related to the addition of the test
> >>builtin_frame_address_test.
> >
> >Thanks for the report.
> >
> >I'm new to arm - will try to setup arm cross-compiler toolchain
> >and see
> >what is going on there, but that will probably take time. In the
> >meantime, I'm having problem to build tcc arm cross compiler on i386:
> 
> I can maybe fix it myself if you have problem setting up your
> environment.

Studying arm was long on my TODO list.

Yesterday evening I've started learning arm instructions. Some more
evenings are probably needed to finish assembly studyings. The next step
would be to setup cross-compile environment which is again time for
probably several evenings. I'm ok with it, but if that's too much time
to wait for others, your help is appreciated.


> >    gcc -o arm-fpa-tcc tcc.c -DONE_SOURCE -DTCC_TARGET_ARM  -Wall -g
> >-O2 -fno-strict-aliasing -Wno-pointer-sign -Wno-sign-compare
> >-Wno-unused-result -mpreferred-stack-boundary=2 -march=i386
> >-falign-functions=0 -m32 -lm -ldl
> >    In file included from libtcc.c:50:0,
> >                     from tcc.c:22:
> >    arm-gen.c: In function ‘gfunc_call’:
> >    arm-gen.c:1058:7: warning: implicit declaration of function
> >‘vfpr’ [-Wimplicit-function-declaration]
> >    In file included from libtcc.c:42:0,
> >                     from tcc.c:22:
> >    tccpp.c: In function ‘macro_subst’:
> >    tccpp.c:2806:12: warning: ‘*((void *)&cval+4)’ is used
> >uninitialized in this function [-Wuninitialized]
> >    /tmp/ccWBLnLc.o: In function `gfunc_call':
> >    /home/kirr/src/tools/tinycc/arm-gen.c:1058: undefined
> >reference to `vfpr'
> >    /home/kirr/src/tools/tinycc/arm-gen.c:1058: undefined
> >reference to `vfpr'
> >    collect2: error: ld returned 1 exit status
> >    Makefile:194: recipe for target 'arm-fpa-tcc' failed
> >    make: *** [arm-fpa-tcc] Error 1
> >
> >
> >could you please fix it, or tell me what is the workaround, so I
> >could
> >try fixing tcc while working on cross-version of it?
> 
> Yes sure, I'm sorry. I broke build without vfp. I always tested with
> VFP in my tests.
> 
> (following up)
> Forgot to mention it's fixed.

Thanks.


> >Also, while I'm studying how to setup arm toolchain, what is
> >`objdump -d`
> >output on arm for gcc and tcc for the next source?
> >
> >---- 8< ----
> >void *bfa0()
> >{
> >    return __builtin_frame_address(0);
> >}
> >
> >void *bfa1()
> >{
> >    return __builtin_frame_address(1);
> >}
> >
> >void *bfa2()
> >{
> >    return __builtin_frame_address(2);
> >}
> >---- 8< ----
> 
> *** tcc ***
> 
> 00000000 <bfa0>:
>    0:   e1a0c00d        mov     ip, sp
>    4:   e92d5800        push    {fp, ip, lr}
>    8:   e28db00c        add     fp, sp, #12
>    c:   e1a00000        nop                     ; (mov r0, r0)
>   10:   e28b0000        add     r0, fp, #0
>   14:   e1a00000        nop                     ; (mov r0, r0)
>   18:   e91ba800        ldmdb   fp, {fp, sp, pc}
> 
> 0000001c <bfa1>:
>   1c:   e1a0c00d        mov     ip, sp
>   20:   e92d5800        push    {fp, ip, lr}
>   24:   e28db00c        add     fp, sp, #12
>   28:   e1a00000        nop                     ; (mov r0, r0)
>   2c:   e59b0000        ldr     r0, [fp]
>   30:   e1a00000        nop                     ; (mov r0, r0)
>   34:   e91ba800        ldmdb   fp, {fp, sp, pc}
> 
> 00000038 <bfa2>:
>   38:   e1a0c00d        mov     ip, sp
>   3c:   e92d5800        push    {fp, ip, lr}
>   40:   e28db00c        add     fp, sp, #12
>   44:   e24bd010        sub     sp, fp, #16
>   48:   e59b0000        ldr     r0, [fp]
>   4c:   e50b0010        str     r0, [fp, #-16]
>   50:   e51be010        ldr     lr, [fp, #-16]
>   54:   e59e0000        ldr     r0, [lr]
>   58:   e1a00000        nop                     ; (mov r0, r0)
>   5c:   e91ba800        ldmdb   fp, {fp, sp, pc}
> 
> *** gcc ***
> 
> 00000000 <bfa0>:
>    0:   b480            push    {r7}
>    2:   af00            add     r7, sp, #0
>    4:   463b            mov     r3, r7
>    6:   4618            mov     r0, r3
>    8:   46bd            mov     sp, r7
>    a:   bc80            pop     {r7}
>    c:   4770            bx      lr
>    e:   bf00            nop
> 
> 00000010 <bfa1>:
>   10:   b480            push    {r7}
>   12:   af00            add     r7, sp, #0
>   14:   683b            ldr     r3, [r7, #0]
>   16:   4618            mov     r0, r3
>   18:   46bd            mov     sp, r7
>   1a:   bc80            pop     {r7}
>   1c:   4770            bx      lr
>   1e:   bf00            nop
> 
> 00000020 <bfa2>:
>   20:   b480            push    {r7}
>   22:   af00            add     r7, sp, #0
>   24:   683b            ldr     r3, [r7, #0]
>   26:   681b            ldr     r3, [r3, #0]
>   28:   4618            mov     r0, r3
>   2a:   46bd            mov     sp, r7
>   2c:   bc80            pop     {r7}
>   2e:   4770            bx      lr


It looks like gcc generated thumb instructions. Could you please ask it
to generate plain arm 4-byte wide assembly, so I can compare tcc & gcc
outputs.

Also, what is the output of tests/tcctests.gcc for
__builtin_frame_address() test? It should be

    str: __builtin_frame_address
    bfa1: __builtin_frame_address
    bfa2: __builtin_frame_address
    bfa3: __builtin_frame_address


> >By the way, what arm variant I should test against - it seems
> >there are
> >many? Another question: maybe you know, what is the most
> >convenient way
> >to setup arm cross-toolchain on Debian GNU/Linux on i386?
> 
> The bug I experienced was with the hardfloat variant (that is,
> TCC_ARM_HARDFLOAT, TCC_ARM_EABI and TCC_ARM_VFP must be defined).
> Maybe the bug also happens on EABI but I can't tell for sure since I
> haven't tried.

Ok, I will be setting up armhf then.


> >Thanks,
> >Kirill
> 
> Thank to you :)

Nevermind :)