From: SourceForge.net <no...@so...> - 2012-02-15 08:38:47
|
Bugs item #3429793, was opened at 2011-10-28 09:15 Message generated for change (Comment added) made by epetrich You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3429793&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: C-Front End >Group: fixed >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Thomas Sailer (tsailer) >Assigned to: Erik Petrich (epetrich) Summary: Reads of volatile variables sometimes optimized away Initial Comment: In the following code, the read of BLABLA is dropped: __xdata volatile unsigned char __at(0x1234) BLABLA; void doit(unsigned char n) { unsigned char i; for (i = 0; i < n; ++i) BLABLA; } I compiled using this command: sdcc -o x.rel -c -mmcs51 x.c When I change BLABLA; to (void)BLABLA; or to {BLABLA;}, or if I change the loop bound (n) to a constant, then the read is retained. Versions of SDCC tested: SDCC : mcs51/gbz80/z80/z180/r2k/ds390/pic16/pic14/TININative/ds400/hc08 3.0.6 #6993 (Oct 26 2011) (Linux) SDCC : mcs51/gbz80/z80/ds390/pic16/pic14/TININative/ds400/hc08 3.0.0 #6037 (Apr 13 2011) (Linux) ---------------------------------------------------------------------- >Comment By: Erik Petrich (epetrich) Date: 2012-02-15 00:38 Message: Fixed in 3.1.2 #7317 ---------------------------------------------------------------------- Comment By: Erik Petrich (epetrich) Date: 2011-11-06 00:11 Message: Using revision 7015, compiling with "sdcc -c -mz80 vol.c --i-code-in-asm" I get: _doit_start:: _doit: push ix ;vol.c:6: for (i = 0; i < 4; ++i) ;ic:2: iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] := 0x4 {const-unsigned-char literal} ld d,#0x04 ;ic:3: _forcontinue_0($3) : 00103$: ;vol.c:7: BLABLA; ;ic:4: dummy = (volatile) _BLABLA [k4 lr0:0 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{volatile-unsigned-char fixed} ;ic:5: iTemp1 [k5 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char auto}[d ] = (const-char register)iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] ;ic:6: iTemp2 [k6 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char register}[d ] = iTemp1 [k5 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char auto}[d ] - 0x1 {const-char literal} dec d ;ic:7: iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] = (unsigned-char auto)iTemp2 [k6 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char register}[d ] ;vol.c:6: for (i = 0; i < 4; ++i) ;ic:9: if iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] != 0 goto _forcontinue_0($3) ld a,d or a, a jr NZ,00103$ ;ic:15: _return($4) : ;ic:16: eproc _doit [k1 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( unsigned-char auto) __reentrant fixed} pop ix ret _doit_end:: Note the ic:4 line. If I compile with "sdcc -c -mz80 vol.c --i-code-in-asm --no-peep" I get: _doit_start:: _doit: push ix ld ix,#0 add ix,sp ;vol.c:6: for (i = 0; i < 4; ++i) ;ic:2: iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] := 0x4 {const-unsigned-char literal} ld d,#0x04 ;ic:3: _forcontinue_0($3) : 00103$: ;vol.c:7: BLABLA; ;ic:4: dummy = (volatile) _BLABLA [k4 lr0:0 so:0]{ ia1 a2p0 re0 rm0 nos0 ru0 dp0}{volatile-unsigned-char fixed} ld iy,#_BLABLA ld a, 0 (iy) ;ic:5: iTemp1 [k5 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char auto}[d ] = (const-char register)iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] ;ic:6: iTemp2 [k6 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char register}[d ] = iTemp1 [k5 lr7:8 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char auto}[d ] - 0x1 {const-char literal} dec d ;ic:7: iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] = (unsigned-char auto)iTemp2 [k6 lr8:9 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{const-char register}[d ] ;vol.c:6: for (i = 0; i < 4; ++i) ;ic:9: if iTemp0 [k2 lr3:10 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{unsigned-char auto}{ sir@ _doit_i_1_1}[d ] != 0 goto _forcontinue_0($3) ld a,d or a, a jp NZ,00103$ ;ic:15: _return($4) : 00104$: ;ic:16: eproc _doit [k1 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( unsigned-char auto) __reentrant fixed} pop ix ret _doit_end:: Which is why I thought a peephole rule was responsible. ---------------------------------------------------------------------- Comment By: Philipp Klaus Krause (spth) Date: 2011-11-05 23:26 Message: Well, for me volatile unsigned char __at(0x1234) BLABLA; void doit(unsigned char n) { unsigned char i; for (i = 0; i < 4; ++i) BLABLA; } does not generate any read iCodes. Philipp P.S.: Can you post the example where the reads are optimized away by z80 peepholes here? ---------------------------------------------------------------------- Comment By: Erik Petrich (epetrich) Date: 2011-11-05 18:48 Message: Philipp, I misread your comment and misunderstood the point of your comment. With a constant in place of n, the iCodes appear to be correct, but the z80 peephole rules are removing the assembly instructions performing the read. Probably the "notVolatile" rule restriction (the version with no argument) needs to be applied to a peephole rule. ---------------------------------------------------------------------- Comment By: Erik Petrich (epetrich) Date: 2011-11-05 17:26 Message: The problem is not so much that the read is optimized away, but that the read is never generated to begin with because of how the symbol is attached to the AST. The routines that generate the iCode for the AST operator nodes NULLOP and BLOCK specially look for a child node that's a volatile symbol and insert a DUMMY_READ_VOLATILE iCode when they find one. In this particular example, the volatile symbol is ending up as a child of a LABEL node where the values returned by the children are discarded and volatile symbols are not treated as a special case. When the loop bound is replaced by a constant, the loop reversing algorithm structures the tree differently and the symbol ends up under a NULLOP node which handles the volatile read correctly. I've verified that the bug also occurs with blockless while and do loops. I tried a fix for this earlier in the week, but it unexpectedly caused some errors building the library functions, so I decided to hold off until after the 3.1.0 release and give the problem a bit more thought in the meantime. ---------------------------------------------------------------------- Comment By: Philipp Klaus Krause (spth) Date: 2011-11-05 12:37 Message: I can reproduce this issue (only removed the __xdata), but for me the bug occours when using a constant instead of n as well. I tried both the mcs51 and z80 ports with current svn. Philipp ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3429793&group_id=599 |