#2045 mcs51 long switch/case blocks cause jmp into oblivion

closed-fixed
5
2013-05-25
2012-06-14
No

I have the following snippet of code at the end of several switch/case blocks:
#ifdef SDCC
/*
* This is here for what appears to be a bug in SDCC.
* I suspect a problem with the jump table optimisation for switch
* statements, the disparate case number will result in less optimised
* code.
* Without this workaround multiple calls to the function lock up the
* controller. The first call always works. As far as I've been able to
* determine the end of the function is reached, but the line after
* the second function call never is.
*/
case 0xFF:
RESET_RMAP();
break;
#endif

It reads a little weird.

My details:
> sdcc -v
SDCC : mcs51/gbz80/z80/z180/r2k/ds390/pic16/pic14/TININative/ds400/hc08 3.1.0 #7066 (Feb 12 2012) (FreeBSD)

Compile:
sdcc -mmcs51 --xram-loc 0xF000 --xram-size 3072 -I inc/ -o bin.sdcc/hsk_flash/hsk_flash.rel -c src/hsk_flash/hsk_flash.c
[...]

Link:
sdcc -mmcs51 --xram-loc 0xF000 --xram-size 3072 -I inc/ -o bin.sdcc/main.hex bin.sdcc/hsk_timers/hsk_timer01.rel bin.sdcc/hsk_icm7228/hsk_icm7228.rel bin.sdcc/hsk_pwc/hsk_pwc.rel bin.sdcc/hsk_pwm/hsk_pwm.rel bin.sdcc/hsk_adc/hsk_adc.rel bin.sdcc/hsk_can/hsk_can.rel bin.sdcc/hsk_boot/hsk_boot.rel bin.sdcc/main.rel bin.sdcc/hsk_isr/hsk_isr.rel bin.sdcc/hsk_flash/hsk_flash.rel

The code runs on the Infineon XC878-16FF.

Discussion

<< < 1 2 3 > >> (Page 2 of 3)
  • Kamikaze Dominic Fandrey

    @epetrich
    I have removed my workaround from all files and can confirm that your peephole rule works. Without it the system freezes, with it works as intended.

    Thank you very much!

    As for making SDCC XC800 compatible, that's the only issue I've encountered so far. Will you add an -mxc800 flag for the xc800 specific peephole rules? It seems a little harsh to inflict the burden of additional cycles on unaffected platforms.

     
  • Kamikaze Dominic Fandrey

    Also, I'm making excessive use of function pointers in ISRs. There is no issue with them. Or if there is, it's masked by the reti.

     
  • Erik Petrich

    Erik Petrich - 2012-10-04

    Calling via function pointer should work fine for functions that have no parameters. In this case, there is a lcall to a library function _sdcc_call_dptr() which then does a jmp to the actual function entry point; this lcall will keep the Memory Extension Stack balanced with the ret at the end of the function.

    The problematic case that's not covered by my peephole rule is using a function pointer with functions that have parameters. As Maarten points out, the above strategy can not be used in this case due to conflicting register needs. So the compiler instead uses the same method that was used to jump to the cases in the larger switch statements and is failing for this microcontroller.

    It looks like currently the function pointers in hsk use no parameters, so the one peephole rule I've given you is sufficient for now. However, I'd like to make sure we also have a working solution for function pointers with parameters so that all the cases are covered. So I will leave this report open a little longer.

    Thanks for confirming that the peephole rule fixed the problem; this was certainly obscure.

     
  • Kamikaze Dominic Fandrey

    quote: epetrich
    ...the XC878 only has a single bank implemented, but the
    documentation shows the banking registers are still there and presumably
    the associated hardware is as well ...

    The banking actually has a purpose. E.g. on the 16-FF variant (64k flash) you can access the XRAM and bootloader from bank 2. So you can execute functions provided by the boot loader and execute code from XRAM. This is useful to bootstrap the µC for live flashing.

    There's probably a way to bank in external RAM/Flash, too. But I never looked into this. 64k of flash are plenty for my purposes (so far I'm only using up to 20k).

     
  • Erik Petrich

    Erik Petrich - 2012-11-17
    • labels: --> mcs51(8051) target
    • status: open --> closed-fixed
     
  • Erik Petrich

    Erik Petrich - 2012-11-17

    There's now (as of revision #8221) a new option for the mcs51 backend "--no-ret-without-call" that should ensure that everywhere that SDCC uses ret to do a computed jump it will also use a call instruction to keep the Memory Extension Stack from underflowing. With this option you should no longer need the custom peephole rule (it's been integrated into the code generator) and it takes care of both the switch/case problem you were seeing as well as the call via function pointer to function with parameters problem that you hadn't encountered yet (and would not have been fixed by the peephole rule)

     
  • Maarten Brock

    Maarten Brock - 2012-11-17

    I think the function pointer case is not fixed yet because there are two RET's involved.

    mov a,#LSB(return_label)
    push acc
    mov a,#MSB(return_label)
    push acc
    push LSB(destination)
    push MSB(destination)
    lcall .+3
    dec sp
    dec sp
    ret
    ---> destination: ret
    return_label:

    I suggest to replace this code with the following which is even smaller!

    lcall make_call
    ljmp return_label
    make_call:
    lcall .+3
    dec sp
    dec sp
    push LSB(destination)
    push MSB(destination)
    ret
    ---> destination: ret
    return_label:

    The labels should all be temporaries of course, but have names for clarity here.
    The peephole optimizer will normally replace LJMP with JR.
    Also by pushing destination after the dummy call there is less stack space needed.

    And even though a little bit bigger I think I prefer to always use JMP @A+DPTR for switch statements.

     
  • Maarten Brock

    Maarten Brock - 2012-11-17
    • status: closed-fixed --> open-fixed
     
  • Maarten Brock

    Maarten Brock - 2012-11-17

    I just had another idea about the parameter passing for function pointers. Currently functions being called with function pointers are required to be reentrant when they have multiple parameters so these are passed on the stack. Only functions with a single parameter can be non-reentrant and still be used for a function pointer because the first parameter is passed in DPTR,B,A.

    Now if we change the calling convention for reentrant functions to pass ALL parameters on stack, we can always use LCALL __sdcc_call_dptr for function pointers. Along with my proposed switch implmentation this means this new command line option can dissapear again.

     
  • Maarten Brock

    Maarten Brock - 2012-11-18

    Should be fixed now in SDCC 3.2.1 #8223.

    I've changed the switch to always use JMP @A+DPTR.
    I've also replaced the pushes for the return address with an LCALL as explained below. This required a bit more work as the peephole optimer didn't recognize it.

    I hope I haven't offended Erik for taking this over and I also hope the users won't mind the slight code increase.

     
<< < 1 2 3 > >> (Page 2 of 3)

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

Sign up for the SourceForge newsletter:





No, thanks