www.digitalmars.com         C & C++   DMDScript  

D.gnu - Accessing peripheral registers - the next version

reply Timo Sintonen <t.sintonen luukku.com> writes:
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?
Aug 27 2016
next sibling parent Mike <none none.com> writes:
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
Aug 27 2016
prev sibling next sibling parent reply Johannes Pfau <nospam example.com> writes:
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;
Aug 28 2016
parent reply Timo Sintonen <t.sintonen luukku.com> writes:
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'.
Aug 28 2016
parent reply Johannes Pfau <nospam example.com> writes:
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.
Aug 28 2016
parent Timo Sintonen <t.sintonen luukku.com> writes:
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.
Aug 31 2016
prev sibling parent reply Timo Sintonen <t.sintonen luukku.com> writes:
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?
Aug 31 2016
parent reply Johannes Pfau <nospam example.com> writes:
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.
Aug 31 2016
next sibling parent reply Timo Sintonen <t.sintonen luukku.com> writes:
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); }
Aug 31 2016
parent Johannes Pfau <nospam example.com> writes:
Am Wed, 31 Aug 2016 12:28:58 +0000
schrieb Timo Sintonen <t.sintonen luukku.com>:


 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); }
 
 


Sorry, it took me some time to get back to this. Anyway, forceinline
works for me if I use it in the same module:
https://paste.gnome.org/p1nlnvagl

If I instead move the main function to a different file, I can
reproduce the error. This is the known cross-module inlining problem.
I'll probably have a look at this soon.
https://forum.dlang.org/post/mzevpbxsyvdrcyjuwhuu forum.dlang.org
Sep 24 2016
prev sibling parent reply Timo Sintonen <t.sintonen luukku.com> writes:
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.
Aug 31 2016
parent Johannes Pfau <nospam example.com> writes:
Am Wed, 31 Aug 2016 19:06:37 +0000
schrieb Timo Sintonen <t.sintonen luukku.com>:


 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.


See https://dlang.org/spec/struct.html#struct-postblit :
"1. blitting the fields, i.e. copying the bits [...] The first part is
done automatically by the language,"

So assume we have a (Volatile!uint)* called 'value' pointing to 0x4.
Volatile!uint is a struct with a uint raw member. Now if we do
something like this:

Volatile!uint copy = *value;

This tells DMD that copy is a struct with one uint field on the stack.
Now to create this stack variable the compiler will copy the 4 bytes

the compiler does not know that the data at 0x4 is 'volatile'. So it
would insert a normal / 'non-volatile' load. And this load is then
subject to the optimizations we want to avoid with Volatile!T.

So we somehow have to prevent the compiler from making such copies. The
only way to do this is  disable this(this).


 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.


I think there's a misunderstanding here:
The problem only occurs if you pass a _Volatile!T_ by value:

void foo(Volatile!uint value) {}
foo(timer.count);

But if you only need the payload data you can pass the _T_ type instead
and everything works fine, even with  disable this(this): The alias
this implements this implicit convertion. This means the compiler will
call load for you when preparing the function arguments (with the
standard D side effect rules, i.e. LTR evaluation)

void foo(uint value) {}
foo(timer.count); // <-- load is called here

If you really want to pass a Volatile!T type (or a struct containing
Volatile!T fields) the only safe way is pass by reference / pointer.
You have ta avoid copying / reading the data, but you can of course
refer to it's memory location / address:

void foo(ref Volatile!uint value) {}
foo(timer.count);


 
 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.
 


That's a valid point, but handling this is a DMD task. Please report
this at issues.dlang.org.
Sep 24 2016