[sdcc-devel] [ sdcc-Bugs-663539 ] volatile does not work

[SourceForge.net <no...@so...>]

Bugs item #663539, was opened at 2003-01-07 08:24
Message generated for change (Comment added) made by maartenbrock
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=663539&group_id=599

Category: Icode generator
Group: known bugs
Status: Open
Resolution: None
Priority: 8
Submitted By: Tzvetan Mikov (mikov)
Assigned to: Erik Petrich (epetrich)
Summary: volatile does not work

Initial Comment:
SDCC version 2.3.0 for 8051:

volatile xdata char x;
void test ( void )
{
  x;
}

This should generate an access to _x_, but it doesn't. It 
is a problem when _x_ is a hardware register.

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

>Comment By: Maarten Brock (maartenbrock)
Date: 2005-03-21 16:27

Message:
Logged In: YES 
user_id=888171

I see genDummyRead in avr, ds390, hc08, mcs51, pic, 
pic16, xa51 and z80. Does this mean every port has this bug 
fixed and this report can be closed? Has anyone any idea for 
a regression test for this bug.

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

Comment By: Erik Petrich (epetrich)
Date: 2003-10-03 09:21

Message:
Logged In: YES 
user_id=635249

Fixed for mcs51, ds390, z80, & their associated targets. See
ChangeLog 1.444

pic & pic16 still need a real genDummyRead and support for
dummy iTemps.

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

Comment By: Tzvetan Mikov (mikov)
Date: 2003-01-14 03:45

Message:
Logged In: YES 
user_id=675109

In this context it is an interresting question whether sfr-s 
should be volatile by default. I think that ideally it should be 
decided on a case by case basis - for example SBUF should 
be declared volatile, but "pure" registers like DPTR, ACC, B, 
etc should not be. 

Volatile has always been a pain in the a** :-) Note that the 
Standard actually allows an implementation to 
ignore "volatile", as long as this is a documented and 
deliberate choice ("What constitutes an access to an object 
that has volatile-qualified type is implementation-defined"). 
Unfortunately this is not very useful in practice...

I am thinking that for now a possibly easy workaround might 
be to ignore "volatile" altogether but add several inline 
intrinsic functions like __volatile_read_char(), 
__volatile_write_char(), etc, with precisely defined semantics. 
This however still could be defeated by a peephole 
optimization like this seemingly safe one:

replace  {
	mov  a,%1
	mov  a,%1
} by {
	mov  a,%1
}

so a predicate "operandsNotVolatile" may have to be added.

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

Comment By: Johan Knol (johanknol)
Date: 2003-01-11 16:17

Message:
Logged In: YES 
user_id=63512

You are right, this is a bug. Sorry about my premature 
conclusion.

The problem starts in ast2iCode where it is NULLOP. This 
can be handled by inserting a dummy assignment to an 
iTemp, however a volatile iTemp is misinterpreted in the 
optimizer and if handled correctly there, later on in the code 
generator. The following code shows that clearly:

sfr at 0x80 MYREG;

void oops (void) {
  MYREG=MYREG;
}

The aopPut() and aopGet() methods in the code generator 
have no knowledge about the volatile-ness of the symbol. This 
requires a major redesign.

So I raised the bug with three points above average.


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

Comment By: Tzvetan Mikov (mikov)
Date: 2003-01-11 03:27

Message:
Logged In: YES 
user_id=675109

SDCC is not correct.

Relevant abstracts from the standard:

5.1.2.3 Program execution

1 The semantic descriptions in this International Standard 
describe the behavior of an abstract machine in which issues 
of optimization are irrelevant.

2 Accessing a volatile object, modifying an object, modifying 
a file, or calling a function that does any of those operations 
are all side effects,11) which are changes in the state of the 
execution environment. Evaluation of an expression may 
produce side effects. At certain specified points in the 
execution sequence called sequence points, all side effects 
of previous evaluations shall be complete and no side effects 
of subsequent evaluations shall have taken place. (A 
summary of the sequence points is given in annex C.)

3 In the abstract machine, all expressions are evaluated as 
specified by the semantics. An actual implementation need 
not evaluate part of an expression if it can deduce that its 
value is not used and that no needed side effects are 
produced (including any caused by calling a function or 
accessing a volatile object).

6.7.3 Type qualifiers

6 An object that has volatile-qualified type may be modified in 
ways unknown to the implementation or have other unknown 
side effects. Therefore any expressionreferring to such an 
object shall be evaluated strictly according to the rules of the 
abstract machine, as described in 5.1.2.3. Furthermore, at 
every sequence point the value last stored in the object shall 
agree with that prescribed by the abstract machine, except 
as modified by the unknown factors mentioned 
previously.114) What constitutes an access to an object that 
has volatile-qualified type is implementation-defined.


In fact GCC does comply with this. GCC 2.95.3 generates 
the following
*correct* code:

volatile int x;
void test ( void )
{
  x;
}

_test:
 pushl %ebp
 movl %esp,%ebp
 movl _x,%eax
 movl %ebp,%esp
 popl %ebp
 ret

If newer versions of GCC behave otherwise, then they are 
broken.

Further on, the proposed solution of using "dummy=x" 
doesn't actually change anything. If "dummy" is a non-volatile 
local variable, then it can be optimized out, bringing us back 
to the same problem. If "dummy" is global variable, then we 
are changing the desired semantics of the program (which in 
this case are to read from "x" and ignore the value), which is 
unacceptable.

This bug may be difficult to fix, but there is no other standard 
way to access memory-mapped hardware registers. I believe 
this is very important for an embedded compiler.

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

Comment By: Johan Knol (johanknol)
Date: 2003-01-10 10:31

Message:
Logged In: YES 
user_id=63512

GCC doesn't do that either, so I think this is correct. Use 
dummy=x instead.

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

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