I updated my system to gdc master. As expected, my program that uses shared variables to accress hardware registers did not work any more. I took the old Volatile datatype we made some years ago and the modified version is something like this:

struct Volatile(T) {
     T raw;

     nothrow:

     @disable this(this);

     A opAssign(A)(A a) { volatileStore(&raw, a); return a; }

     T load() @property { return volatileLoad(&raw); }

     alias load this;

     void opOpAssign(string OP)(const T rhs) {
          auto v = volatileLoad(&raw);
          mixin("v " ~ OP ~ "= rhs;");
          volatileStore(&raw, v);
     }
}

I did some simple tests and this seems to work.

My question is: is this the way we should go?
If it is, what would be the preferred name for this type? Also what would you think should be the module and file name?

On Saturday, 27 August 2016 at 08:05:08 UTC, Timo Sintonen wrote:

> My question is: is this the way we should go?
> If it is, what would be the preferred name for this type? Also what would you think should be the module and file name?

There was some discussion about the module and file names when `volatileLoad` and `volatileStore` were added to the runtime.
(https://github.com/dlang/druntime/pull/1032#issuecomment-64071568).

It was generally agreed upon that volatile features should go in `core.volatile` after the `volatile` keyword was removed.  Seems reasonable to me.
(https://issues.dlang.org/show_bug.cgi?id=13826).

The `volatile` keyword was removed in March 2016.
(https://github.com/dlang/dmd/pull/5556)

`core.volatile` seems like a good place for this `Volatile(T)` implementation, IMO.

Mike

Am Sat, 27 Aug 2016 08:05:08 +0000
schrieb Timo Sintonen <t.sintonen@luukku.com>:

> [...]
> I did some simple tests and this seems to work.
> 
> My question is: is this the way we should go?

Yes, I think this is the best way to implement a volatile type. It is a natural replacement for volatile variables as it can be used in exactly the same way:

1) For variables, used e.g. in interrupts and in the normal program code:

Volatile!bool dataAvailable;

2) For memory mapped registers:

(cast((Volatile!uint)*)0xABCD)* += 1;


The alternative is a type which also internally handles the address of
the 'data'. However, then you have one template instance per
address. With your solution there's one template instance per type
which is more reasonable IMO.

The Volatile!T code you posted does not show how to map such a Volatile!T instance to a certain address. The code in 2) could be placed into a @property, but some stuff might still not work correctly (like using & to get the address). I think I'll add a @attribute("address", 0xABCD) to GDC which can be used for all extern variables. Then you could simply do this:

extern Volatile!int PORTA @address(0xABCD);

> If it is, what would be the preferred name for this type? Also what would you think should be the module and file name?
> 

As Mike already posted, Volatile!T is a good name (as this exactly explains what the type does for anyone already knowing the C volatile meaning).

However, for accessing peripheral registers I'd use something more high
level and name it Register!(...) or something like that. Here's a proof
of concept:
https://github.com/D-Programming-microD/avr-playground/blob/master/src/test.d
https://github.com/D-Programming-microD/avr-playground/blob/master/src/util/mmio.d

// using enums for fields
DDRB.pin1 = Direction.out_;
PORTB.pin1 = Level.low;
PORTB.toggle!"Pin1";
PORTB.set!("Pin1", "Pin2");
PORTB.clear!("Pin1", "Pin2");
PORTB.set!("Pin1", Level.low,
           "Pin2", Level.high);

// classic, c-style usage
DDRB |= (0b1 << DDRB.shiftPin1) | ( 0b1 << DDRB.shiftPin2);
DDRB = 0b1;

On Sunday, 28 August 2016 at 08:34:15 UTC, Johannes Pfau wrote:
> Am Sat, 27 Aug 2016 08:05:08 +0000
> schrieb Timo Sintonen <t.sintonen@luukku.com>:
>
>
> The Volatile!T code you posted does not show how to map such a Volatile!T instance to a certain address. The code in 2) could be placed into a @property, but some stuff might still not work correctly (like using & to get the address). I think I'll add a @attribute("address", 0xABCD) to GDC which can be used for all extern variables. Then you could simply do this:
>
> extern Volatile!int PORTA @address(0xABCD);
>
I just translated my sample program and everything seems to work in my limited tests. Here is a simplified example of an uart:

alias uarttype = uartreg*;
enum uarttype uart1=cast (uarttype)0x40011000;
enum uarttype uart2=cast (uarttype)0x40004400;

alias console = uart1;

struct uartreg {

   Volatile!uint sr;
   Volatile!uint dr;
   Volatile!uint brr;
   Volatile!uint cr1;
   Volatile!uint cr2;
   Volatile!uint cr3;
   Volatile!uint gtpr;


   // send a byte to the uart
   void send(int t) {
     while ((sr&0x80)==0) // wait for tx ready flag
     {  }
     dr=t;  // write to data register
   }

}

void test() {
  foreach(c;" abcde ")
	console.send(c);
}


>> what would be the preferred name for this type? Also what would you think should be the module and file name?
>> 
>
> As Mike already posted, Volatile!T is a good name (as this exactly explains what the type does for anyone already knowing the C volatile meaning).
>
> However, for accessing peripheral registers I'd use something more high
> level and name it Register!(...) or something like that.

Then let it be Volatile and we can alias that later.
Because GDC is only at 2.068, it seems we can not name the module as 'volatile'.

Am Sun, 28 Aug 2016 09:28:24 +0000
schrieb Timo Sintonen <t.sintonen@luukku.com>:

> I just translated my sample program and everything seems to work in my limited tests. Here is a simplified example of an uart:
> 
> alias uarttype = uartreg*;
> enum uarttype uart1=cast (uarttype)0x40011000;
> enum uarttype uart2=cast (uarttype)0x40004400;
> 

That's a clever solution. AFAICS it works because D supports
the '.' operator on pointers. With this solution you can't directly
assign the complete value:

uart1 = uart2; // can't be valid, assigning pointers
*uart1 = *uart2; // should work, though IIRC there could be a DMDFE bug

IIRC you also cant use operator overloading:

uart1 += 49; // can't be valid, assigning the pointer
*uart1 += 49; // might work? (not sure if dereferencing
              // and overloads work in same statement)


And you can't use the & operator to get the address, but as uart1 already is a pointer that's not really a problem.


In your example this isn't really a restriction. But it could be more annoying if you don't have related fields with contiguous addresses. Consider a single 8 bit counter value:

alias CtrType = (Volatile!ubyte)*; enum CtrType ctrA=cast (CtrType)0x40011000;

*ctrA += 3;
*ctrA = 42;

OTOH this is a nice solution without language changes, so @attribute("address") is probably not necessary. I think it'd be a little more idiomatic to just treat memory mapped registers as extern variables, but maybe it's not worth the effort.


> Then let it be Volatile and we can alias that later.
> Because GDC is only at 2.068, it seems we can not name the module
> as 'volatile'.
> 

Even DMD 2.071 doesn't allow 'volatile' as a module name :-) We need a DMD patch first and then we could backport that patch to GDC if necessary.

On Sunday, 28 August 2016 at 13:26:37 UTC, Johannes Pfau wrote:
> Am Sun, 28 Aug 2016 09:28:24 +0000
> schrieb Timo Sintonen <t.sintonen@luukku.com>:
>
>> I just translated my sample program and everything seems to work in my limited tests. Here is a simplified example of an uart:
>> 
>> alias uarttype = uartreg*;
>> enum uarttype uart1=cast (uarttype)0x40011000;
>> enum uarttype uart2=cast (uarttype)0x40004400;
>> 
>
> That's a clever solution. AFAICS it works because D supports
> the '.' operator on pointers. With this solution you can't directly
> assign the complete value:
>
> uart1 = uart2; // can't be valid, assigning pointers
> *uart1 = *uart2; // should work, though IIRC there could be a DMDFE bug
>
> IIRC you also cant use operator overloading:
>
> uart1 += 49; // can't be valid, assigning the pointer
> *uart1 += 49; // might work? (not sure if dereferencing
>               // and overloads work in same statement)
>
>
> And you can't use the & operator to get the address, but as uart1 already is a pointer that's not really a problem.
>
One reason I like D is that enums can have type. I can write functions that take that type as an argument.
Having these as compile time constant means less indirection: no need to take an address of a variable that holds the pointer to the struct.
Peripherals are always in fixed addresses and have fixed names. There is no need to create a struct of this type or address them in different places or use pointers in calculations.


>
> In your example this isn't really a restriction. But it could be more annoying if you don't have related fields with contiguous addresses. Consider a single 8 bit counter value:
>
> alias CtrType = (Volatile!ubyte)*; enum CtrType ctrA=cast (CtrType)0x40011000;
>
> *ctrA += 3;
> *ctrA = 42;
>

Addressing single locations is more difficult. Pointers are not the preferred way in D. Individual registers are quite rare in microcontrollers and it is always possible to have a struct with only one volatile member.

Some thing that I have noticed in the original struct definition

> struct Volatile(T) {
>      T raw;
>      nothrow:
>      @disable this(this);
>      A opAssign(A)(A a) { volatileStore(&raw, a); return a; }
>      T load() @property { return volatileLoad(&raw); }
>      alias load this;
>
>      void opOpAssign(string OP)(const T rhs) {
>           auto v = volatileLoad(&raw);
>           mixin("v " ~ OP ~ "= rhs;");
>           volatileStore(&raw, v);
>      }
> }

There was @disable this(this)
This prevents to use a location as argument to a function, like: print(timer.count). Is there any reason to have this line or not? What about disable this() ? I will never need to create any instance of this type.

OpAssign did originally return void. Then I had a piece of code like cr1=cr2=cr3=0 which did not work. Now I return the same type but I think I have seen that it should return this. What should it return and would it make shorter code if it does not return anything?

The original functions had force_inline attribute. The compiler said it can not inline because the body is not available. Is this because they are templates or is this because of the intrinsic call? These calls are often in time critical places and inlining them is very important.

When these are all templated, all modules have their own instances of all templates for all data and operator types. That is lots of extra code. There are intrinsics for only four data types. Should I make overloads for these 4 types instead of templates? These functions are so short that code duplication does not matter.

For the outer struct (like uartregs struct in my example) there is compiler generated stuff like opEquals and toHash. I will never need them and they take a lot of space. How can I disable them?

Am Wed, 31 Aug 2016 09:07:49 +0000
schrieb Timo Sintonen <t.sintonen@luukku.com>:

> Some thing that I have noticed in the original struct definition
> 
> > struct Volatile(T) {
> >      T raw;
> >      nothrow:
> >      @disable this(this);
> >      A opAssign(A)(A a) { volatileStore(&raw, a); return a; }
> >      T load() @property { return volatileLoad(&raw); }
> >      alias load this;
> >
> >      void opOpAssign(string OP)(const T rhs) {
> >           auto v = volatileLoad(&raw);
> >           mixin("v " ~ OP ~ "= rhs;");
> >           volatileStore(&raw, v);
> >      }
> > }
> 
> There was @disable this(this)
> This prevents to use a location as argument to a function, like:
> print(timer.count). Is there any reason to have this line or not?

I guess timer is a (enum) pointer to a struct and count is a Volatile!T
field in that struct?
But then timer.count does not return a address, it returns the value of
count? Unless of course print(X) takes its argument by ref or alias.

The reason for @disable this(this) is to disable copying of the struct.
The compiler otherwise accesses T raw in a 'non-volatile' way when
copying the value. In D we can only have a postblit function, but we
cannot reimplement the copying. opAssign can only be used in some cases:
    Foo f;
    auto f2 = Foo();
    auto f3 = f2;
    f = Foo();
    f = f2;

Only the last two lines call opAssign. So "auto f3 = f2" could copy a Volatile!T in a non-volatile way. This is the reason for @disable this(this), AFAIK.

> What about disable this() ? I will never need to create any instance of this type.

The only usecase are variables for interrupts AFAIK:

shared|__gshared Volatile!bool sendDone = false;

@interrupt(Interrupt.uartReady) void onUartReady()
{
    sendDone = true;
}

void startSendData(ubyte[]...)
{
    sendDone = false;
    ...
    while(!sendDone)
}

> 
> OpAssign did originally return void. Then I had a piece of code like cr1=cr2=cr3=0 which did not work. Now I return the same type but I think I have seen that it should return this. What should it return and would it make shorter code if it does not return anything?
Both cases are valid D code, AFAIK. But let's consider this example cr1=cr2=0:

First of all, there are two possible, different interpretations of the code: Assign 0 to cr2 and 0 to cr1 OR assign 0 to cr2 and cr2 to cr1. For non-shared, non-volatile variables this is the same. But for shared or volatile variables the result could be different.

Consider this example is lowered into
cr2 = 0;
cr1 = cr2;

This is difficult to get right:
1) Volatile!T opAssign(...) {return this;}
   Is wrong: The "return this" part will do a non-volatile read/copy.
2) ref Volatile!T opAssign(...) {return this;}
   Should be correct. But then the example above will additionally
   produce a volatile read from cr2 when setting cr1. This implements
   the second behaviour mentioned above.
3) T opAssign(T rhs){return rhs} implements behaviour 1 mentioned above.

I think this is too confusing so we should return void and simply
disallow the pattern (AFAICS it also doesn't work with @disable
this(this), though the could be a DMD bug).

> 
> The original functions had force_inline attribute. The compiler said it can not inline because the body is not available. Is this because they are templates or is this because of the intrinsic call? These calls are often in time critical places and inlining them is very important.
> 

This is probably a old GDC/DMDFE bug. Cross-module inlining (i.e. function with force-inline in one module, caller in another module) wasn't possible with older frontend versions. It should be possible now, but it is not implemented in GDC. Can you post a simple test case? Usually templated methods are the only methods which should work with cross module inlining.

> When these are all templated, all modules have their own instances of all templates for all data and operator types. That is lots of extra code. There are intrinsics for only four data types. Should I make overloads for these 4 types instead of templates? These functions are so short that code duplication does not matter.

Those templates are emitted to every module, but they should have the same name and be marked as weak, so the linker should merge them into a single instance. So as long as Volatile!T is restricted to ubyte, ushort, uint, ulong* there shouldn't be additional bloat.

* I wonder whether using Volatile!T with types larger than the
  atomically readable type is useful. Probably not.

> For the outer struct (like uartregs struct in my example) there is compiler generated stuff like opEquals and toHash. I will never need them and they take a lot of space. How can I disable them?

AFAIK there's no way to disable this function. Extending @disable to these functions (=> @disable toHash...) should hopefully be uncontroversial, but needs to be done in DMD first.

On Wednesday, 31 August 2016 at 10:12:21 UTC, Johannes Pfau wrote:
> Am Wed, 31 Aug 2016 09:07:49 +0000
> schrieb Timo Sintonen <t.sintonen@luukku.com>:
>
>
>> 
>> The original functions had force_inline attribute. The compiler said it can not inline because the body is not available. Is this because they are templates or is this because of the intrinsic call? These calls are often in time critical places and inlining them is very important.
>> 
>
> This is probably a old GDC/DMDFE bug. Cross-module inlining (i.e. function with force-inline in one module, caller in another module) wasn't possible with older frontend versions. It should be possible now, but it is not implemented in GDC. Can you post a simple test case? Usually templated methods are the only methods which should work with cross module inlining.
>
Anything that access the Volatile members:

int test()
{ return uart3.sr; }

error: inlining failed in call to always_inline 'load': function body not available
T load @property { return volatileLoad(&raw); }

On Wednesday, 31 August 2016 at 10:12:21 UTC, Johannes Pfau wrote:

>> There was @disable this(this)
>> This prevents to use a location as argument to a function, like:
>> print(timer.count). Is there any reason to have this line or not?
>
> I guess timer is a (enum) pointer to a struct and count is a Volatile!T
> field in that struct?
Yes

> But then timer.count does not return a address, it returns the value of
> count? Unless of course print(X) takes its argument by ref or alias.
I want to use them just by value.

>
> The reason for @disable this(this) is to disable copying of the struct.
> The compiler otherwise accesses T raw in a 'non-volatile' way when
> copying the value. In D we can only have a postblit function, but we
> cannot reimplement the copying. opAssign can only be used in some cases:
>     Foo f;
>     auto f2 = Foo();
>     auto f3 = f2;
>     f = Foo();
>     f = f2;
>
> Only the last two lines call opAssign. So "auto f3 = f2" could copy a Volatile!T in a non-volatile way. This is the reason for @disable this(this), AFAIK.
>
I think I somehow get the point even if I do not fully understand the explanation.
But it is too restrictive if I always need to have a getter function or assign the value first to a temporary variable to use the value in a function call.

Anyway, I think it is a gdc bug that the error message comes from the called function that has nothing to do with this. The error should show the function call where the disabled copy operation is.