Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#2287 Inline functions with variable argument list fails on MCS-51

open
nobody
None
MCS51
5
2014-08-15
2014-07-21
Ben Shi
No

The attached .c file failed on mcs-51-stack-auto, but succeeded on mcs-51-small.

If the static prefixes of f1i() and f2i() were removed, then they became ordinary functions and the inline made no effect, but succeeded on both ports.

1 Attachments

Discussion

  • Maarten Brock
    Maarten Brock
    2014-08-13

    I have a lot of questions about this bug report.

    First, it states this report was updated 22 hours ago. But by whom? And how? I don't get it.

    Second, I don't like the way the test file is modified in the regression test suite to work around this bug. It is just changed into a whole different source by removing the inline keywords. That's no fix. And apart from the subversion history there's nothing indicating how it was when it failed.

    Third, I can't seem to find the original gcc test that this file should originate from. When I search the internet for stdarg-4.c I only find files without inline functions.

    And finally it is unclear how the attached file actually fails. I looked through the generated asm for f1() and see no problem. So which of the 9 ASSERTions fails or does it does it produce build errors?

    Maarten

     
  • Maarten Brock
    Maarten Brock
    2014-08-13

    Ah wait, I see, the file is duplicated in gcc-torture-execute-stdarg-5.c. We have other mechanisms for duplicating files with minor differences. It should use a keyword with values in the first comment like this:

    /*
       stdarg-4.c from the execute part of the gcc torture tests.
       attr: static inline, 
     */
    {attr} void
    f1i (va_list ap)
    
     
  • Ben Shi
    Ben Shi
    2014-08-13

    As far as I remembered,

    The original stdarg-4.c failed in host test with newer gcc (>= 4.7) if --std-c99 was specified, since a non-static inline function was wiped out in the optimization but inline code insertion was not performed. Then I added statics to all inlines in rev9051, but quickly found that this way succeeded on all ports except the mcs51-auto-stack. So in rev9052 I changed functions in stdarg-4.c to ordinary ones while created a new stdarg-5.c with statics before all inlines.

    The error is easy to reproduce by removing the #if !defined(SDCC_mcs51) in stdarg-5.c.

    Since I did not know the rule of duplicating a test case then, you are appreciated to correct my mistake.

     
  • Ben Shi
    Ben Shi
    2014-08-14

    If the #if !defined(__SDCC_mcs51) was removed, the gcc-torture-execute-stdarg-5.c rose error in mcs51-auto-stack on my 64-bit MacOS 10.9.4. The attachment is the error message, the .c and the generated .asm.

     
    Attachments
  • Maarten Brock
    Maarten Brock
    2014-08-14

    And once again something weird happened. I received an email from SF with a reply from Ben in this thread that is now gone. Have you deleted it afterwards Ben and sent the last reply with the zip instead?

    A quote from your 'deleted' reply:

    gen/mcs51-stack-auto/gcc-torture-execute-stdarg-5/gcc-torture-execute-stdarg-5.out:3:--- FAIL: "Assertion failed" on 0 at gen/mcs51-stack-auto/gcc-torture-execute-stdarg-5/gcc-torture-execute-stdarg-5.c:132
    Failure: gen/mcs51-stack-auto/gcc-torture-execute-stdarg-5/gcc-torture-execute-stdarg-5
    

    This once again proves that all those ASSERT(0) are less efficient. I still have to find the right line which changed due to the removed #define line. And the message says nothing about which test failed because that is hidden in the surrounding if-statement.

    if (x != 100L || y != 30L)
        ASSERT (0);
    

    It would have been so much better if the two tests were there directly and in two in separate ASSERTs.

    ASSERT (x == 100L);
    ASSERT (y == 30L);
    
     
  • Maarten Brock
    Maarten Brock
    2014-08-14

    At least now I see what's going wrong. It totally forgets to assign to y in f2(). It seems to forget that y is global.

    ;   -----------------------------------------
    ;    function f2
    ;   -----------------------------------------
    _f2:
        push    _bp
        mov a,sp
        mov _bp,a
        add a,#0x04
        mov sp,a
    ;   gcc-torture-execute-stdarg-4.c:49: va_start (ap, i);
    ;   sequence point 31
        mov a,_bp
        add a,#0xFC
        mov r7,a
    ;   gcc-torture-execute-stdarg-4.c:39: y = va_arg (ap, int);
    ;   sequence point 32
        dec r7
        dec r7
    BUG: what about y ???
    ;   gcc-torture-execute-stdarg-4.c:40: y += va_arg (ap, long);
        dec r7
        dec r7
        dec r7
        dec r7
    BUG: what about y ???
    ;   gcc-torture-execute-stdarg-4.c:41: y += va_arg (ap, double);
        dec r7
        dec r7
        dec r7
        dec r7
    BUG: what about y ???
    ;   gcc-torture-execute-stdarg-4.c:21: x = va_arg (ap, double);
        dec r7
        dec r7
        dec r7
        dec r7
    CORRECT: x is assigned
        mov ar6,r7
        mov ar1,r7
        mov ar3,@r1
        inc r1
        mov ar4,@r1
        inc r1
        mov ar5,@r1
        inc r1
        mov ar7,@r1
    ;   sequence point 33
        mov dpl,r3
        mov dph,r4
        mov b,r5
        mov a,r7
        push    ar6
        lcall   ___fs2slong
        mov _x,dpl
        mov (_x + 1),dph
        mov (_x + 2),b
        mov (_x + 3),a
        pop ar6
    
     
  • Maarten Brock
    Maarten Brock
    2014-08-14

    The code is removed between DUMP_RAW1 and DUMP_CSE in eBBlockFromiCode() in SDCCopt.c as can be seen from these dumps:

    DUMP_RAW1:

    gcc-torture-execute-stdarg-4.c(l46:s0:k1:d0:s0)     proc _f2 [k1 lr0:0 so:5]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( int auto) __reentrant fixed}
    gcc-torture-execute-stdarg-4.c(l49:s0:k2:d0:s0)     iTemp1 [k5 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int near* fixed} = &[i [k4 lr0:0 so:-4]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{int auto} , 0x0 {const-unsigned-char literal}]
    gcc-torture-execute-stdarg-4.c(l49:s0:k3:d0:s0)     iTemp2 [k6 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed} = (unsigned-char near* fixed)iTemp1 [k5 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int near* fixed}
    gcc-torture-execute-stdarg-4.c(l49:s0:k4:d0:s0)     iTemp0 [k2 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22} := iTemp2 [k6 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}
    gcc-torture-execute-stdarg-4.c(l50:s0:k5:d0:s0)     iTemp3 [k7 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2___00020005_3_26} := iTemp0 [k2 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22}
    gcc-torture-execute-stdarg-4.c(l50:s0:k6:d0:s0)     iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} := iTemp3 [k7 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2___00020005_3_26}
    gcc-torture-execute-stdarg-4.c(l39:s0:k7:d0:s0)     iTemp5 [k12 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed} = iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} - 0x2 {unsigned-char literal}
    gcc-torture-execute-stdarg-4.c(l39:s0:k8:d0:s0)     iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} := iTemp5 [k12 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}
    8<----------------- the following lines disappeared !
    gcc-torture-execute-stdarg-4.c(l39:s0:k9:d0:s0)     iTemp6 [k13 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int near* fixed} = (int near* fixed)iTemp5 [k12 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}
    gcc-torture-execute-stdarg-4.c(l39:s0:k10:d0:s0)        iTemp7 [k14 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int register} = @[iTemp6 [k13 lr0:0 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{int near* fixed} + 0x0 {const-unsigned-char literal}]
    gcc-torture-execute-stdarg-4.c(l39:s0:k11:d0:s0)        iTemp8 [k15 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{long-int fixed} = (long-int fixed)iTemp7 [k14 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int register}
    gcc-torture-execute-stdarg-4.c(l39:s0:k12:d0:s0)        iTemp9 [k16 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{long-int fixed} := iTemp8 [k15 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{long-int fixed}
    gcc-torture-execute-stdarg-4.c(l39:s0:k13:d0:s0)        _y [k11 lr0:0 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{long-int fixed} := iTemp9 [k16 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{long-int fixed}
    8<-----------------
    gcc-torture-execute-stdarg-4.c(l40:s0:k14:d0:s0)        iTemp10 [k17 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed} = iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} - 0x4 {unsigned-char literal}
    


    DUMP_CSE:

    gcc-torture-execute-stdarg-4.c(l46:s0:k1:d0:s0)     proc _f2 [k1 lr0:0 so:5]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( int auto) __reentrant fixed}
    gcc-torture-execute-stdarg-4.c(l49:s0:k2:d0:s0)     iTemp1 [k5 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int near* fixed} = &[i [k4 lr0:0 so:-4]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{int auto} , 0x0 {const-unsigned-char literal}]
    gcc-torture-execute-stdarg-4.c(l49:s0:k3:d0:s0)     iTemp2 [k6 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22} = (unsigned-char near* fixed)iTemp1 [k5 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int near* fixed}
    gcc-torture-execute-stdarg-4.c(l49:s0:k4:d0:s0)     iTemp0 [k2 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22} := iTemp2 [k6 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22}
    gcc-torture-execute-stdarg-4.c(l50:s0:k5:d0:s0)     iTemp3 [k7 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2___00020005_3_26} := iTemp0 [k2 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22}
    gcc-torture-execute-stdarg-4.c(l50:s0:k6:d0:s0)     iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} := iTemp0 [k2 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22}
    gcc-torture-execute-stdarg-4.c(l39:s0:k7:d0:s0)     iTemp5 [k12 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} = iTemp0 [k2 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_1_22} - 0x2 {unsigned-char literal}
    gcc-torture-execute-stdarg-4.c(l39:s0:k8:d0:s0)     iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} := iTemp5 [k12 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26}
    gcc-torture-execute-stdarg-4.c(l40:s0:k14:d0:s0)        iTemp10 [k17 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near* fixed} = iTemp4 [k9 lr0:0 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near* fixed}{ sir@ _f2_ap_3_26} - 0x4 {unsigned-char literal}
    


    Most probably it happens here:

    computeDataFlow (ebbi);
    killDeadCode (ebbi);
    
     
  • Ben Shi
    Ben Shi
    2014-08-14

    The attachment in my previous reply was broken, so I deleted it and resent again with a new zip file.

    Can you reproduce this bug at your side? It is easy to be produced both on my 32-bit arm board and my 64-bit MacOS 10.9.4.

    If it was due to arch-independant optimization, why other ports are OK with this test case? Such as mcs51-small and mcs51-large. But only mcs51-auto-stack rises errors.

     
  • Ben Shi
    Ben Shi
    2014-08-14

    In the original version of gcc-torture-execute-stdarg-4.c, only inlines but no static were specified. And I have checked the generated code of host, stm8 and mcs51-*, that

    1. SDCC omitted the inline keywords and the inline functions were ordinary ones.
    2. GCC 4.7 above and llvm rose syntax errors.
     
  • Ben Shi
    Ben Shi
    2014-08-14

    Sorry, GCC 4.7 above and llvm rose link time error. The expected code inlining did not happen in the caller, and the caller still held a call instruction to the inline callee. While the inline callee was wiped out.

     
  • Ben Shi
    Ben Shi
    2014-08-15

    Should the computeDataFlow() and killDeadCode() be arch independent? I compiled this .c with --i-code-in-asm for mcs51-stack-auto and mcs51-small, and also found the written back to the global variable y was missing in mcs51-stack-auto while was present in mcs51-small.

    mcs51-small:
    ; q.c:38: y = va_arg (ap, int);
    ; [-------7] ic:5: iTemp5 [k12 lr5:7 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near fixed}[r7 ] = iTemp0 [k2 lr4:5 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near fixed}{ sir@ _f2_ap_1_17}[r7 ] - 0x2 {unsigned-char literal}
    dec r7
    dec r7
    ; [------67] ic:6: iTemp4 [k9 lr6:15 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near fixed}{ sir@ _f2_ap_3_21}[r6 ] := iTemp5 [k12 lr5:7 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near fixed}[r7 ]
    mov ar6,r7
    ; [-1----67] ic:7: iTemp6 [k13 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int near fixed}[r1 ] = (int near fixed)iTemp5 [k12 lr5:7 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{unsigned-char near fixed}[r7 ]
    mov ar1,r7
    ;
    [-1---567] ic:8: iTemp7 [k14 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int register}[r5 r7 ] = @[iTemp6 [k13 lr7:8 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{int near
    fixed}[r1 ] + 0x0 {const-unsigned-char literal}]
    mov ar5,@r1
    inc r1
    mov ar7,@r1
    ; [-----567] ic:9: _y [k11 lr0:0 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{long-int fixed} = (long-int fixed)iTemp7 [k14 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int register}[r5 r7 ]
    mov _y,r5
    mov a,r7
    mov (_y + 1),a
    rlc a
    subb a,acc
    mov (_y + 2),a
    mov (_y + 3),a
    ; q.c:39: y += va_arg (ap, long);

    mcs51-auto-stack:

    ; q.c:38: y = va_arg (ap, int);
    ; [-------7] ic:5: iTemp4 [k9 lr5:8 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near fixed}{ sir@ _f2_ap_3_21}[r7 ] = iTemp0 [k2 lr4:5 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char near fixed}{ sir@ _f2_ap_1_17}[r7 ] - 0x2 {unsigned-char literal}
    dec r7
    dec r7
    ; q.c:39: y += va_arg (ap, long);

     

    Related

    Commit: [r1]
    Commit: [r6]
    Commit: [r7]