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

closed-fixed
Erik Petrich
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 1 of 3)
  • What I forgot, I cannot debug this, because the debugger looses contact to the µC when the issue occurs.

     
  • Maarten Brock
    Maarten Brock
    2012-06-22

    How are we to check this bug if you only provide small snippets of code?
    Can you please attach an example file that reproduces the bug you mentioned?

     
  • Hmm, that's strange, I never got an e-mail that there was a comment.

    I'll wrap something up in a couple of days.

     
  • Example code, returns right into oblivion after the second hsk_ex_port_open(call)

     
    Attachments
  • Erik Petrich
    Erik Petrich
    2012-09-29

    • milestone: --> non_bugs
    • assigned_to: nobody --> epetrich
     
  • Erik Petrich
    Erik Petrich
    2012-09-29

    I've carefully looked through this for many hours and as far as I can tell, SDCC's handling of the switch statement is correct for an 8051/2 compatible processor.

    I suspect the problem you are seeing is due to a slight incompatibility introduced with how the XC800 series processors implement their banked memory support (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 processor has a secondary stack (Memory Extension Stack) and stack pointer register (MEXSP) that are updated by the LCALL, ACALL, RET, and RETI instructions. This allows the processor to seamlessly transition between functions spread over a 1M address space (again, I realize the XC878 only implements a small portion of this memory).

    In the switch statements that you've encountered problems, SDCC has pushed the address of the appropriate case onto the regular stack and then executed a RET instruction. The RET instruction forms an address from the top two bytes popped from the regular stack and then jumps to this address. However, the XC800 series additionally pops a byte from the Memory Extension Stack and stores this to the MEX1 register, which potentially changes the currently selected bank of memory. The problem is that this RET instruction is not matched with a previous ACALL or LCALL instruction that would have pushed the correct byte onto the Memory Extension Stack (and since this does not exist on an 8051/2, SDCC has no idea that this needs to be taken into account).

    This is consistent with your description of the function working correctly on the first invocation, but failing to return on the second. There has been a call from the start-up code to main(), from main() to hsk_ex_port_open(), and perhaps another function in between. So by the time the switch statement is processed, there are several bytes on the Memory Extension Stack. The RET instructions in the two switch statements each take a byte of this stack, as does the RET at the end of the function. Although the MEX1 register is being reloaded in each instance, the value is always the same (indicating bank 0 is selected) so the correct code continues to execute. However, after the first call to hsk_ex_port_open(), MEXSP is pointing closer to the start of the Memory Extension Stack than it should. During the second invocation, MEXSP underflows past the start of the Memory Extension Stack, and MEX1 is loaded with some indeterminate value while processing one of the RET instructions, thus selecting a bank of memory that does not exist.

    SDCC uses a similar code sequence to invoke a function via a function pointer and would probably result in the same sort of crash.

    I think you could probably make SDCC compatible with the XC800 series by using a few custom peephole rules. This one should keep the Memory Extension Stack from underflowing during a switch statement:
    replace {
    push acc
    ret
    } with {
    push acc
    lcall .+3
    dec sp
    dec sp
    ret
    }

    I think the dispatching via a function pointer could be handled similarly, but I've been staring at this too long today to attempt a peephole rule for that right now. Please try out the above peephole rule and confirm whether or not it fixed the problem you are seeing.

     
  • Erik Petrich
    Erik Petrich
    2012-09-29

    Oops, that peephole rule should be:
    replace {
    push acc
    ret
    } by {
    push acc
    lcall .+3
    dec sp
    dec sp
    ret
    }

     
  • Will try this next week (I'm visiting my folks and didn't take any hardware with me). And thanks for the insightful reply!

     
  • Maarten Brock
    Maarten Brock
    2012-09-29

    Hmm, this is another good reason to want to use JMP @A+DPTR in crtcall.asm for all function pointers. But that requires ACC and DPTR to be free for the call and currently they're used for passing the first parameter. Thus crtcall.asm can only used for (void) functions.

    The jump table probably can always use JMP @A+DPTR but will give slightly larger code. The <112 case can use MOV DPL/DPH,A instead of PUSH ACC, but requires an extra CLR A. The <256 case requires PUSH ACC and an extra POP DPL on top of that. I have no objections against such a modification.

    Maarten

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