RE: [PATCH][RFC] Remove volatile from data members in libstdc++

["Boehm, Hans" <hans dot boehm at hp dot com>]

> -----Original Message-----
> From: Richard Guenther [mailto:rguenther@suse.de] 
> 
> On Wed, 12 Jul 2006, Richard Guenther wrote:
> 
> > > 4) At some point atomicity.h was added.  It operates on 
> volatiles.  
> > > It has to, since it doesn't provide an atomic read 
> operation, and I 
> > > suspect a bunch of clients read the values directly.  Thus if it 
> > > didn't operate on volatiles it might actually yield incorrect 
> > > results on common platforms; this way it's not in any 
> sense standard 
> > > conforming, but probably works.
> > 
> > It doesn't operate on volatiles.  It simply has volatile qualified 
> > function parameters to avoid warnings if it happens to operate on 
> > volatiles.
Good point.  I agree.
> > Also you don't need atomic read operations (there is no 
> > such thing really), instead you have to work out your reference 
> > counting scheme correctly, as you mentioned above.  
> Volatile doesn't 
> > help if that is wrong.
I disagree.  It doesn't help if you atomically increment or otherwise
update an int, and then the compiler generates two short reads to read
it back.  We do agree that volatile still doesn't provide a guarantee
that this won't happen.  But it does 

a) Tell the compiler that asynchronous changes are possible, and hence
hopefully reduces the probability of performing transformations that are
unsafe in their presence, and

b) Warn the programmer that something fishy (an intentional data race)
is going on here.

As far as (a) is concerned, I'm also worried about more complex compiler
transformations.  For example, without the volatile declaration, in some
circumstances it is acceptable for the compiler to read the reference
count more than once if it has to be spilled from a register.  Thus the
same unmodified const local copy could appear to contain two different
values in different places.  I really don't want to have to reason about
such things.

I'm not arguing that the volatile qualifier makes things 100% correct;
it doesn't.  I am saying it's more informative and reduces breakage
probability until we have 100% correct solution, which we currently
don't.  (We could get closer at a large performance cost by consistently
using locks to protect the reference counts.  But that's not acceptable.
And unfortunately even then the state of threads standards is currently
such that you don't get 100% guarantees.)

As far as I can tell, the patch is otherwise a great improvement.  Aside
from the removal of the volatile declaration from the ref_count field, I
strongly support it.

If we don't want volatile declarations in such code, probably an even
better solution is to add simple read and write accessors to atomicity.h
(whose default implementation may just cast to a volatile pointer).  I'm
very nervous about removing the volatile declarations without putting
something else in their place that warns the compiler and reader of the
data race.

This latter alternative would also be consistent with what's being
discussed in the C++ committee.

(There has been some discussion on the C++ committee as to whether
volatile should have Java semantics, and hence become usable for
inter-thread communication.  My current guess is that won't happen.  See
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2016.html.)

Hans