Erik and Maarten,

please update the sdccman.lyx with recent enhancements: new command line option and introduction of a stand alone sdas390.

Borut

Dne 2012 11 17 07:51 je "SourceForge.net" <noreply@sourceforge.net> napisal/-a:
Bugs item #3535290, was opened at 2012-06-14 13:43
Message generated for change (Comment added) made by epetrich
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3535290&group_id=599

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
>Category: mcs51(8051) target
Group: non bugs
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Kamikaze Dominic Fandrey (lonkamikaze)
Assigned to: Erik Petrich (epetrich)
Summary: mcs51 long switch/case blocks cause jmp into oblivion

Initial Comment:
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.

----------------------------------------------------------------------

>Comment By: Erik Petrich (epetrich)
Date: 2012-11-16 22:51

Message:
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)

----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-10-10 07:38

Message:
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).

----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2012-10-03 21:06

Message:
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.


----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-10-03 15:38

Message:
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.

----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-10-03 09:04

Message:
@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.

----------------------------------------------------------------------

Comment By: Maarten Brock (maartenbrock)
Date: 2012-09-29 04:59

Message:
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

----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-09-29 03:11

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

----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2012-09-29 00:07

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


----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2012-09-28 23:55

Message:
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.

----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-09-27 13:07

Message:
Well, finally there's some affected code online:
https://sourceforge.net/p/hsk/libs/ci/cb8d8415c84b2178fdd5115437fdfb6cd2f290ff/tree/src/hsk_ex/hsk_ex.c#l243

----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-08-22 00:50

Message:
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.

----------------------------------------------------------------------

Comment By: Maarten Brock (maartenbrock)
Date: 2012-06-22 04:51

Message:
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?

----------------------------------------------------------------------

Comment By: Kamikaze Dominic Fandrey (lonkamikaze)
Date: 2012-06-14 13:44

Message:
What I forgot, I cannot debug this, because the debugger looses contact to
the C when the issue occurs.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3535290&group_id=599

------------------------------------------------------------------------------
Monitor your physical, virtual and cloud infrastructure from a single
web console. Get in-depth insight into apps, servers, databases, vmware,
SAP, cloud infrastructure, etc. Download 30-day Free Trial.
Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
sdcc-devel mailing list
sdcc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sdcc-devel