Menu
▾
▴
[sdcc-devel] [ sdcc-Bugs-1618050 ] badly optimized xdata pointer
|
From: SourceForge.net <no...@so...> - 2007-07-08 13:25:48
|
Bugs item #1618050, was opened at 2006-12-18 05:59 Message generated for change (Comment added) made by nobody You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1618050&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: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: wek (wek_) Assigned to: Nobody/Anonymous (nobody) Summary: badly optimized xdata pointer Initial Comment: SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.6.1 #4478 (Nov 23 2006) (MINGW32) When compiling the attached snippet for mcs51 (using plainly "sdcc.exe testbug.c"), both *px translates to mov dptr,#_x movx a,@dptr i.e. *x, which may not be true. In this short test program, I am unable to insert some code modifying px before "if", to demonstrate that the "if" is not redundant, without "spoiling the error", but I have a long-long program where this really happens. Jan Waclawek ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2007-07-08 06:25 Message: Logged In: NO this patch seems to be a temporary workaround --------------------------- --- sdcc-4870/src/ds390/ralloc.c 2007-06-28 11:18:31.000000000 +0200 +++ sdcc-4875-all/src/ds390/ralloc.c 2007-07-08 13:36:37.000000000 +0200 @@ -3065,18 +3065,18 @@ packRegisters (eBBlock ** ebpp, int bloc /* if straight assignment then carry remat flag if this is the only definition */ if (ic->op == '=' && !POINTER_SET (ic) && IS_SYMOP (IC_RIGHT (ic)) && OP_SYMBOL (IC_RIGHT (ic))->remat && !IS_CAST_ICODE(OP_SYMBOL (IC_RIGHT (ic))->rematiCode) && bitVectnBitsOn (OP_SYMBOL (IC_RESULT (ic))->defs) <= 1) - { + if(0) { // FIXME disabled due to bug 1618050 OP_SYMBOL (IC_RESULT (ic))->remat = OP_SYMBOL (IC_RIGHT (ic))->remat; OP_SYMBOL (IC_RESULT (ic))->rematiCode = OP_SYMBOL (IC_RIGHT (ic))->rematiCode; } /* if cast to a generic pointer & the pointer being cast is remat, then we can remat this cast as well */ --- sdcc-4870/src/mcs51/ralloc.c 2007-06-28 11:16:35.000000000 +0200 +++ sdcc-4875-all/src/mcs51/ralloc.c 2007-07-08 13:35:22.000000000 +0200 @@ -3000,18 +3000,18 @@ packRegisters (eBBlock ** ebpp, int bloc /* if straight assignment then carry remat flag if this is the only definition */ if (ic->op == '=' && !POINTER_SET (ic) && IS_SYMOP (IC_RIGHT (ic)) && OP_SYMBOL (IC_RIGHT (ic))->remat && !IS_CAST_ICODE(OP_SYMBOL (IC_RIGHT (ic))->rematiCode) && bitVectnBitsOn (OP_SYMBOL (IC_RESULT (ic))->defs) <= 1) - { + if(0) { // FIXME disabled due to bug 1618050 OP_SYMBOL (IC_RESULT (ic))->remat = OP_SYMBOL (IC_RIGHT (ic))->remat; OP_SYMBOL (IC_RESULT (ic))->rematiCode = OP_SYMBOL (IC_RIGHT (ic))->rematiCode; } /* if cast to a generic pointer & the pointer being cast is remat, then we can remat this cast as well */ --- sdcc-4870/src/z80/ralloc.c 2007-06-28 11:17:41.000000000 +0200 +++ sdcc-4875-all/src/z80/ralloc.c 2007-07-08 13:38:22.000000000 +0200 @@ -2974,17 +2974,17 @@ packRegisters (eBBlock * ebp) /* Safe: just propagates the remat flag */ /* if straight assignment then carry remat flag if this is the only definition */ if (ic->op == '=' && !POINTER_SET (ic) && IS_SYMOP (IC_RIGHT (ic)) && OP_SYMBOL (IC_RIGHT (ic))->remat && bitVectnBitsOn (OP_SYMBOL (IC_RESULT (ic))->defs) <= 1) - { + if(0) { // FIXME disabled due to bug 1618050 OP_SYMBOL (IC_RESULT (ic))->remat = OP_SYMBOL (IC_RIGHT (ic))->remat; OP_SYMBOL (IC_RESULT (ic))->rematiCode = OP_SYMBOL (IC_RIGHT (ic))->rematiCode; } /* if the condition of an if instruction is defined in the --------------------------------- not only mcs51 suffers from the problem, but z80 and ds390 as well, and only those. i've run support/regression with this additional test file ---------------------------------- bug1618050b.c /** bug 1618050 * global variable px in function foo, is marked rematr.. * and is erronously assumed to retain its value "x" across * the function call to fiddle_px */ #include <testfwk.h> data char * data px; data char x[2] = {0,42}; void fiddle_px(data char * unused) { (volatile char) unused[0]; // shut up px++; } char buggy(void) { px = x; fiddle_px(x); return *px; } void testBug(void) { ASSERT(buggy() == 42); } ------------------------ unfortunatly the patch lets creep in another serious problem, but only for ds390 Processing bitfields.c gen/ds390/bitfields/bitfields.out:5:--- FAIL: "Assertion failed" on size2b_bf.b0==1 at gen/ds390/bitfields/bitfields.c:226 gen/ds390/bitfields/bitfields.out:6:--- FAIL: "Assertion failed" on size2b_bf.b0==1 at gen/ds390/bitfields/bitfields.c:239 Robert Larice larice 0x40 vidisys 0x2e de ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2007-07-08 01:40 Message: Logged In: NO the offending piece of code can be reduced, for debugging purpose to ----------------------- data char * data px; data char x; void fiddle_px(data char * data); char foo(void) { px = &x; fiddle_px(&x); return *px; } --------------------- the generated code looks like this: ; bug-8b.c:11: px = &x; mov _px,#_x ; bug-8b.c:13: fiddle_px(&x); mov dpl,#_x lcall _fiddle_px ; bug-8b.c:15: return *px; mov dpl,_x ret demonstrating, the compiler erronously assumes px can't change across the function call there is no commandline switch from the --no.* familly which avoids this bug. ------------------------- debugging the function packRegisters in src/mcs51/ralloc.c reveals, in /* if straight assignment then carry remat flag if this is the only definition */ if (ic->op == '=' && !POINTER_SET (ic) && IS_SYMOP (IC_RIGHT (ic)) && OP_SYMBOL (IC_RIGHT (ic))->remat && !IS_CAST_ICODE(OP_SYMBOL (IC_RIGHT (ic))->rematiCode) && bitVectnBitsOn (OP_SYMBOL (IC_RESULT (ic))->defs) <= 1) { OP_SYMBOL (IC_RESULT (ic))->remat = OP_SYMBOL (IC_RIGHT (ic))->remat; OP_SYMBOL (IC_RESULT (ic))->rematiCode = OP_SYMBOL (IC_RIGHT (ic))->rematiCode; } the variable px on the left side of the assignment will be marked *remat* gdb call PICC(ic) before that sequence shows (l11:s4:k4:d0:s0) _px [k2 lr0:0 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{char data-near* } := iTemp0 [k4 lr3:5 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{char data-near* } (l13:s5:k6:d0:s0) send iTemp0 [k4 lr3:5 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{char data-near* }{argreg = 1} (l13:s6:k7:d0:s0) iTemp3 [k8 lr6:6 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void} = call _fiddle_px [k6 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( char near* ) } (l15:s7:k8:d0:s0) iTemp4 [k9 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{char data-near* } := _px [k2 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{char data-near* } (l15:s8:k9:d0:s0) iTemp5 [k10 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{data-char} = @[iTemp4 [k9 lr7:8 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{char data-near* }] (l15:s9:k10:d0:s0) ret iTemp5 [k10 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{data-char} and behind that sequence (l11:s4:k4:d0:s0) _px [k2 lr0:0 so:0]{ ia1 a2p0 re0 rm1 nos0 ru0 dp0}{char data-near* } := iTemp0 [k4 lr3:5 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{char data-near* } (l13:s5:k6:d0:s0) send iTemp0 [k4 lr3:5 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{char data-near* }{argreg = 1} (l13:s6:k7:d0:s0) iTemp3 [k8 lr6:6 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void} = call _fiddle_px [k6 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( char near* ) } (l15:s7:k8:d0:s0) iTemp4 [k9 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{char data-near* } := _px [k2 lr0:0 so:0]{ ia0 a2p0 re0 rm1 nos0 ru0 dp0}{char data-near* } (l15:s8:k9:d0:s0) iTemp5 [k10 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{data-char} = @[iTemp4 [k9 lr7:8 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{char data-near* }] (l15:s9:k10:d0:s0) ret iTemp5 [k10 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{data-char} that is _px is now marked rm1 when i disable this if block, the compiler produces correct code. i suppose this "if" needs another condition, checking whether there is a function call in the liverange of the variable. Robert Larice larice 0x40 vidisys 0x2e de ---------------------------------------------------------------------- Comment By: Frieder Ferlemann (frief) Date: 2006-12-19 03:45 Message: Logged In: YES user_id=589052 Originator: NO the attached file "bug1618050.c" allows to reproduce the bug within the regression test suite. All targets except host and hc08 fail. File Added: bug1618050.c ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1618050&group_id=599 |