[sdcc-devel] [ sdcc-Bugs-3429793 ] Reads of volatile variables sometimes optimized away

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

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: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Thomas Sailer (tsailer)
Assigned to: Nobody/Anonymous (nobody)
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: 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

[sdcc-devel] [ sdcc-Bugs-3429793 ] Reads of volatile variables sometimes optimized away

The Small Device C Compiler (SDCC), targeting 8-bit architectures

[sdcc-devel] [ sdcc-Bugs-3429793 ] Reads of volatile variables sometimes optimized away