[dmd-internals] Regarding deprecation of volatile statements

[Artur Skawina]

On 08/01/12 19:20, Alex Rønne Petersen wrote:
> On Wed, Aug 1, 2012 at 1:41 PM, Artur Skawina <art.08.09 at gmail.com> wrote:
>> D's volatile statements were a mistake; since they are already
>> deprecated and obviously flawed, I never saw the need to actually
>> even mention this; i was just waiting until they are gone, so that
>> a sane 'volatile' can be introduced. Your suggestion has the same
>> problems; it does not help, but instead would make fixing the language
>> harder (by keeping the current broken incarnation of volatile around).
> 
> Please explain how they were a mistake. I have seen this "feature X

I meant "were a mistake" as an objective description of state - the
concept of 'volatile statements' is flawed, and they are already
being deprecated, hence the past tense. What the actual reasons
for their removal from the language were, only Walter can answer that.

> was a mistake, so we removed it" thing way too often... and the
> deprecation page on dlang.org is certainly not helpful in explaining
> this either...

Yes, there's way to little both documentation and discussion happening,
which leads to broken or incomplete features being adopted. This is
also why I'd have preferred to discuss /language/ features in the open,
a compiler specific list means that the topic won't get enough attentions.

> Please also explain what you mean by "current incarnation of
> volatile". The C volatile? Or the D volatile statement (which is
> currently deprecated)?

The current deprecated D volatile statement.

>> The issues w/ volatile statements are really obvious, eg
>>
>> What does this do?
>>
>>    C c; D d;
>>    //...
>>    volatile d.i = c.i++;
>>    //...
> 
> I don't see what's wrong here.
> 
> First, the value of c.i is read and saved into a compiler-generated
> temporary. Then, d.i is set to this temporary. Then the temporary is
> incremented and stored into c.i.

The store to 'd.i' in this model *has* to be handled as volatile, which
is not necessarily what you want. Note that using an explicit temporary
variable has the same problem - the store to that one has to be 'volatile'.
It's not possible to express *just* a 'raw' load or store, you always
get them in pairs. One way to uncouple the ops would be to allow

   d.i = volatile { c.i++; } // or d.i = volatile (c.i++);

ie 'volatile expressions'. But then what do you do with stores?

   volatile { c.i = --d.i }   // nope, here '--d.i' was not meant to be
                              // constrained by 'volatile'.

   volatile { c.i } = --d.i;  // Not really, it is no longer the assignment
   volatile (c.i) = --d.i;    // that is 'volatile', it's the /type/ of c.i;

so you're left with

   auto temp = --d.i; volatile c.i = temp;

which isn't nice and it gets much worse with more complex expressions, as 
you then have to split out everything but the 'volatile' part.

Also, 'volatile statements' aren't transitive. So 

   volatile x = c.i;

works (modulo the above issues), but as soon as you need to do

   auto f(ref C c) { if (some_debug_check_etc) {}; return c.i; }
   volatile x = f(c);

the 'volatile' is silently dropped; so you need to always remember
to give c.i special treatment. You get no compiler support; just a
subtle and potentially hard to find bug if you miss just one case.


> I can only guess, but is the problem you're trying to point out that
> there might be multiple reads from c.i depending on the compiler
> implementation? If so, I already mentioned that this is insignificant:
> Excessive reads have no impact on semantics, but writes do.

When dealing with hardware, reads /can/ matter - eg a mmapped interrupt
register can clear itself after every read access - extra loads that
aren't properly handled because they weren't expected are a problem.


>> What about this?
>>
>>    int e;
>>    volatile e = c.i;
> 
> Fetches c.i and stores it into e? Can you be clearer about what's
> wrong here? I don't see the problem. According to my proposal, all the
> volatile would do is ensure that the e = c.i statement isn't moved
> around with respect to other volatile statements, or folded into other
> volatile operations.

The store to 'e' is *not* supposed to be 'volatile' - 'e' just holds
the value that was read from 'c.i'. It has a data dependency on the 
result of the 'c.i' expression, but does *not* need any further
restrictions; it can be placed in a register, and it can even be
completely eliminated as dead code, if that would be the case - it's
just the 'c.i' /load/ that's special. 


>> Even introducing volatile /expresions/ wouldn't solve the problem,
>> it's just the wrong tool for the job. 'volatile' is a property of
>> the data (access), not of expressions/statements.
> 
> I can't really respond to this without some clarification of the above.

See above.


> I think the idea that volatile is a part of the data access is just an
> idea somehow carried over from C. Who says that's the right way? Why
> do we have to do it the way C does it?

We can learn from the decades of C evolution. Volatile statements were
a dead end, that couldn't have worked. Twenty years ago I wouldn't
have trusted C's 'volatile'; these days given a sane compiler and
reasonable expectations (no volatile-bitfields ops etc) doing that
is possible. So, while I can understand Walter's position, i think D
can define a sane 'volatile' type attribute (it's effectively already
part of 'shared', which btw would gain from splitting up into the
individual attributes that 'shared' is composed of). The fact that
there is only one D compiler certainly helps, the specs can be updated
as issues are found, and if another compiler decides to wrongly handle
'volatile' - oh well. It's not likely to happen IMO; not initially
handling 'volatile' at all /is/ possible, but doing it incorrectly 
on purpose wouldn't really make sense...

>> Now imagine if 'C.i' was marked with a 'volatile' attribute. Both
>> of the above examples would get sane semantics, which otherwise
>> can only be approximated using explicit temporary dummy variables.
> 
> Please clarify what is insane about the above examples.

It's not clear which parts the 'volatile' applies to, so the only
possible (sane) interpretation is that it applies to the whole
statement. Which is not what you'd usually want and prevents many
optimizations.

> Which is how almost all compiler IRs do it. You'll rarely find
> compiler IRs that don't use explicit load and store instructions. And,
> after all, defining volatile semantics is also a matter of
> practicality for compiler engineers.

Explicit loads and explicit stores - those are /separate/ operations,
the result of a forced (re)load does not need to be forcibly stored.

>> 'volatile' statements are just as broken as 'shared' statements
>> would be:
>>
>>    shared { a = b; } // which access is the shared one? both?
> 
> I don't see what's odd about this at all. It would be equivalent to:
> 
> atomicStore(&a, atomicLoad(&b));
> 
> (Memory fences omitted.)

Yep, but I want to atomically read 'b' and store the result in 'a', 
what now? I can't, unless i can somehow mark just 'b' as shared.
Which is exactly the same as the 'volatile' situation.
(Like i've said in the past - 'shared' can be used as a 'volatile'
substitute in D, but it's not really the ideal solution)


>> Your arguments in the DIP againts a C-like attribute are equally
>> valid against the "rather nonsensical" volatile statements examples:
>>
>>    int i;
>>
>>    volatile
>>    {
>>       i = 1;
>>       i = 2;
>>    }
> 
> It would be if you think about volatile as a modifier of data access
> semantics. But note that in DIP17, it's more of a constraint on
> execution order in general.
> 
> I think I should have been clearer about that. The way I'd like to see
> volatile is that it modifies the order in which execution must happen,
> not just memory loads and stores.

I still fail to see any difference between the C example and the above.
There are two possibilities
a) 'volatile' is ignored for cases like this (local, unobservable changes)
b) it's exactly the same as marking 'i' as volatile, just restricted to
   a certain scope. In fact the latter seems problematic in itself - do
   you really need/want only /some/ accesses to 'i' to be less optimized?

>> which, btw, are not entirely nonsensical, as you may want i's on-stack
>> (or -heap in case of closures) representation to be kept updated at all
>> times. Which isn't a common requirement, but there is no reason to disallow
>> this usage. It can obviously be expressed using compiler barriers too, but
>> having every access automatically treated specially is much better than
>> having to always remember to mark it as 'volatile' everywhere or wrap it.
>> Think templates, etc.
> 
> Right, that's why I generalized volatile as something higher level
> than a data access modifier: It opens the door for much better control
> than the C volatile.

No, it does the opposite - you need special code for every single case
where 'volatile' accesses are required.

And if you're already need to do that, then just wrapping the accesses
is safer, and likely simpler too. In GDC-speak:

   T volatile_load(T)(ref T v) { // [1]
      asm { "" : "+m" v; }
      T res = v;
      asm { "" : "+g" res; }
      return res;
   }

   T volatile_store(T)(ref T v, const T a) {
      asm { "" : : "m" v; }
      v = a;
      asm { "" : "+m" v; }
      return a;
   }

   void main() {
      int i = 0;

      auto a = volatile_load(i);

      volatile_store(i, 42);
   }

will do the right thing. You can even do

    auto volatile_op(string op, T)(ref T v) {
      auto a = volatile_load(v);
      auto res = mixin(op);
      volatile_store(v, a);
      return res;
   }

   volatile_op!"a++"(i);

and this will compile into the expected load+addition+store.

And it's portable across platforms (obviously not compilers, until
there's a common standard); Each arch would just need versions with
the necessary mem fences, if it needs them.

Wrapping things like mmapped IO/files using this kind of helpers (or
intrinsics if the compilers asm support doesn't let you express what
you need) would be a good idea anyway.

> But that is also why making a variable volatile and forcing all
> accesses to be such is limiting. You won't be able to access it in a
> non-volatile way should you so wish.

Pointers. Casts. Etc.

artur

[1]
   // Note: the above versions are 'unordered'; if you
   // need to ensure the order of /unrelated/ accesses
   // something like this will work; it's not always
   // required and more costly.

   T volatile_load_ordered(T)(ref T v) {
      asm { "" : "+m" v : : "memory"; }
      T res = v;
      asm { "" : "+g" res : : "memory"; }
      return res;
   }