#74 excessive push/pop between function calls

closed
None
5
2006-12-26
2004-06-04
Anonymous
No

I has found several mistakes in SDCC:

Source:

#include <8052.h>

unsigned char test;

void test1(void)
{
//do something...
}

void test2(void)
{
//do something
}

void test3(void)
{
unsigned char tmp;
for (tmp=0; tmp!=5; tmp++)
{
test1();
test2();
test1();
}
}

void main(void)
{
test3();
}

SDCC run command:

sdcc --verbose --model-small --peep-asm -mmcs51 --
iram-size 256
--xram-size 0 --code-size 12288 --nojtbound test.c

SDCC version:

SDCC :
mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51
/ds400/hc08 2.4.1 (Jun 3 2004) (MINGW32)

Asm list:
388 ; ---------------------------
--------------
389 ; function test3
390 ; ---------------------------
--------------
0033 391 _test3:
392 ;test.c:18: for (tmp=0; tmp!
=5; tmp++)
393 ; genAssign
0033 7A 00 394 mov r2,#0x00
0035 395 00101$:
396 ; genCmpEq
0035 BA 05 01 397 cjne
r2,#0x05,00109$
398 ; Peephole 112.b changed
ljmp to sjmp
399 ; Peephole 251.b replaced
sjmp to ret with ret
0038 22 400 ret
0039 401 00109$:
402 ;test.c:20: test1();
403 ; genCall
0039 C0 02 404 push ar2
003B 12s00r31 405 lcall _test1
003E D0 02 406 pop
ar2 <=====
407 ;test.c:21: test2
(); |spare operations with stack
408 ;
genCall |
0040 C0 02 409 push
ar2 <=====
0042 12s00r32 410 lcall _test2
0045 D0 02 411 pop
ar2 <=====
412 ;test.c:22: test1
(); |spare operations with stack
413 ;
genCall |
0047 C0 02 414 push
ar2 <=====
0049 12s00r31 415 lcall _test1
004C D0 02 416 pop ar2
417 ;test.c:18: for (tmp=0; tmp!
=5; tmp++)
418 ; genPlus
419 ; genPlusIncr
004E 0A 420 inc r2
421 ; Peephole 112.b changed
ljmp to sjmp
004F 80 E4 422 sjmp 00101$
0051 423 00105$:
0051 22 424 ret

Discussion

  • Yee_Keat Phuah

    Yee_Keat Phuah - 2004-07-12

    Logged In: YES
    user_id=940150

    tmp is assigned in r2 in test3, and all the functions called
    by test3, might use r2 for other optimizations, hence test3
    pushes r2 (or ar2) on the stack whenever it calls other
    functions. So I think this is normal.

    If you wishes to optimize it so that it only push/pop ar2
    once in the whole loop, then its a different story, please
    clarify.

     
  • Maarten Brock

    Maarten Brock - 2004-08-11
    • summary: SDCC MCS51 code generation bug --> excessive push/pop between function calls
     
  • Maarten Brock

    Maarten Brock - 2004-08-11

    Logged In: YES
    user_id=888171

    Nice feature request, no bug

     
  • Frieder Ferlemann

    Logged In: YES
    user_id=589052

    > Nice feature request, no bug

    Yes, nice feature request! The following code more pointedly
    shows "excessive push/pop before and after function calls":

    ----8<-------------------------------
    volatile unsigned char in;
    volatile unsigned char out;

    unsigned char f(void)
    {
    return 1;
    }

    unsigned char g(void)
    {
    return f();
    }

    unsigned char main(void)
    {
    /* have some variables assigned to registers */
    unsigned long a = in;
    unsigned int b = in;
    unsigned char c = in;
    unsigned char d = in;

    out = f(); /* all registers pushed although f()
    clobbers none */

    // out += f();

    out += a+b+c+d;
    return out;
    }
    ---->8-------------------------------

    ----8<-------------------------------
    ;saveregister.c:18: out = f(); /* all registers pushed
    although f() clobbers none */
    ; genCall
    push ar2
    push ar3
    push ar4
    push ar5
    push ar6
    push ar7
    push ar0
    push ar1
    lcall _f
    mov _out,dpl
    pop ar1
    pop ar0
    pop ar7
    pop ar6
    pop ar5
    pop ar4
    pop ar3
    pop ar2
    ---->8-------------------------------

    Uncomment the other call of f() for the "excessive push/pop
    _between_ function calls" (noted as "different story" by kiwlm).

     
  • Bernhard Held

    Bernhard Held - 2006-09-27
    • assigned_to: nobody --> bernhardheld
     
  • Bernhard Held

    Bernhard Held - 2006-12-26

    Logged In: YES
    user_id=203539
    Originator: NO

    Fixed in rev. 4526.

    I first thought about a modification of the register allocator (RA). The RA should estimate, if a variable is often pushed/popped. If this is true, the variable isn't assigned to registers. But the current approach via the peephole optimizer is so efficient, that 1. modifiaction of the RA is no longer necessary and 2. the estimations of the RA will utterly wrong after the peephole optimization.

    I still think about a new keyword. C knows the keyword 'register'. But I'm missing a keyword, which tells the compiler to not assign a variable to registers. Any suggestions?

     
  • Bernhard Held

    Bernhard Held - 2006-12-26
    • status: open --> closed
     
  • Maarten Brock

    Maarten Brock - 2006-12-26

    Logged In: YES
    user_id=888171
    Originator: NO

    I think volatile will do the trick.

     
  • Bernhard Held

    Bernhard Held - 2006-12-30

    Logged In: YES
    user_id=203539
    Originator: NO

    You're right, thanks for the hint.
    I was thinking about a modifier in order to hint the compiler to generate better code. But "volatile" will disable many optimizations, which leeds to worse code.
    Anyway "volatile" is good for testing purposes.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks