From: SourceForge.net <no...@so...> - 2007-01-19 10:13:59
|
Bugs item #1638622, was opened at 2007-01-18 15:14 Message generated for change (Comment added) made by wek_ You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1638622&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: msc51(8051) target Group: None Status: Deleted Resolution: None Priority: 5 Private: No Submitted By: sbeaman (sbeaman) Assigned to: Nobody/Anonymous (nobody) Summary: Compiler Generates Improper code Initial Comment: Compiling example code from Silicon Labs I encountered two issues where the compiler seems to be generating improper code. The code should swap the MSB and LSB bytes of an unsigned int. This does not happen. I attached an example program that reproduces the issue. Also note the line in the sample code: test = SETUP.wValue.i generates improper code. Both test and SETUP.wValue.i are unsigned integers. The assembly generated for the assignment is: MOV A,0AH MOV A,0BH Note: 0AH is the address of SETUP.wValue.i Using version 2.6.0 of SDCC. ---------------------------------------------------------------------- Comment By: wek (wek_) Date: 2007-01-19 11:13 Message: Logged In: YES user_id=1201677 Originator: NO bernhardheld wrote: >> 1) Does not generate code to do the swap correctly. > Please be more specific. If you want a better code for the swap have a > look at the fine manual. This made me curious. Is there any way to produce better code for swap (and similar swap-like operations) other than asm? Swap might be potentially a frequently used operation when interfacing to different-endian peripherals and/or data source (e.g. networks)... What am I missing? Jan Waclawek ---------------------------------------------------------------------- Comment By: Frieder Ferlemann (frief) Date: 2007-01-19 00:24 Message: Logged In: YES user_id=589052 Originator: NO added some test code for this in the regression test suite ( http://svn.sourceforge.net/viewvc/sdcc/trunk/sdcc/support/regression/tests/swap.c?view=markup ) Found nothing unusual there (checked with recent svn version). > But it still puzzles me why the optimizer (when test is not volatile) is > moving a 16 bit number twice into an 8 bit register. As Maarten put it: "... "SETUP" is volatile ..." ---------------------------------------------------------------------- Comment By: sbeaman (sbeaman) Date: 2007-01-18 23:59 Message: Logged In: YES user_id=1694917 Originator: YES My bad on the MSB swap. You are correct, the improper definition of LSB and MSB basically negated the swap. Sorry about that. Declaring test as volatile did force it to generate correctly as: ; C:/develop/mymods/C8051/test/badcode/badcode.c:59: test = SETUP.wValue.i; ; genPointerGet ; genNearPointerGet ; genDataPointerGet mov _Test_test_1_1,(_SETUP + 0x0002) mov (_Test_test_1_1 + 1),((_SETUP + 0x0002) + 1) as apposed to the old code (when test is not volatile) of: ; C:/develop/mymods/C8051/test/badcode/badcode.c:59: test = SETUP.wValue.i; ; genPointerGet ; genNearPointerGet ; genDataPointerGet mov a,(_SETUP + 0x0002) mov a,((_SETUP + 0x0002) + 1) But it still puzzles me why the optimizer (when test is not volatile) is moving a 16 bit number twice into an 8 bit register. It would make sense if: - It moved it into two registers and forgot about it, since it is not referenced later. or - Didn't generate any code if it decides the line is useless. Note: Both test and SETUP.wValue.i are 16 bit numbers. ---------------------------------------------------------------------- Comment By: sbeaman (sbeaman) Date: 2007-01-18 23:00 Message: Logged In: YES user_id=1694917 Originator: YES The generated .asm file is attached. Note: even if the endian swap is not required the compiler: 1) Does not generate code to do the swap correctly. 2) Improperly assigns the value to the variable test(as mentioned in the original post) File Added: badcode.asm ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2007-01-18 20:32 Message: Logged In: YES user_id=888171 Originator: NO Jan is right. "test" is only written but never read. It is optimized out. But "SETUP" is volatile so it must be read and thus it's bytes are read into A. The next instructions read SETUP.wValue.c[MSB], cast it to int and store it in r2(lsb) and r3(msb). Because MSB is wrongly defined as 0 this is actually the lsb of SETUP.wValue.i. Then SETUP.wValue.c[LSB] is read into r4 and next multiplied by 256 and stored in r4(lsb) and r5(msb). Finally addition takes place and the result is stored in SETUP.wValue.i. In conclusion the swap is not correctly generated because LSB and MSB are defined wrong for a little-endian compiler and test is not assigned at all. Maarten ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2007-01-18 18:34 Message: Logged In: YES user_id=203539 Originator: NO I can't find a bug too. > 1) Does not generate code to do the swap correctly. Please be more specific. If you want a better code for the swap have a look at the fine manual. ---------------------------------------------------------------------- Comment By: wek (wek_) Date: 2007-01-18 17:04 Message: Logged In: YES user_id=1201677 Originator: NO 1. Why do you think the swap is incorrect? It is lengthy, but IMHO correct. 2. Variable test is never ever used hence optimalisation might result in any garbage which in fact does nothing (exactly as the C line does nothing too). Try to declare test as volatile. Jan Waclawek ---------------------------------------------------------------------- Comment By: sbeaman (sbeaman) Date: 2007-01-18 16:43 Message: Logged In: YES user_id=1694917 Originator: YES The generated .asm file is attached. Note: even if the endian swap is not required the compiler: 1) Does not generate code to do the swap correctly. 2) Improperly assigns the value to the variable test(as mentioned in the original post) File Added: badcode.asm ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2007-01-18 15:58 Message: Logged In: YES user_id=888171 Originator: NO S, You did not supply the generated code, only the C source which explicitly states it's for Keil. SDCC is little-endian by itself (no swap needed). Keil is big-endian and thus requires the swap. Maarten ---------------------------------------------------------------------- Comment By: sbeaman (sbeaman) Date: 2007-01-18 15:23 Message: Logged In: YES user_id=1694917 Originator: YES Forgot to mention the file is compiled with the options: c:\SDCC\BIN\SDCC.EXE -c --debug --use-stdout -V -IC:\develop\mymods\C8051\test\BadCode C:\develop\mymods\C8051\test\BadCode\badcode.c and linked with: c:\SDCC\BIN\SDCC.EXE --debug --use-stdout -V -IC:\develop\mymods\C8051\test\BadCode -oC:\develop\mymods\C8051\test\BadCode\badcode.hex C:\develop\mymods\C8051\test\BadCode\badcode.rel Using Silicon Labs IDE V2.82 default settings for SDCC ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1638622&group_id=599 |