volatile als member variable nicht möglich?

[hustbaer]

Also ich würde bei memory-mapped Registern (bzw. auch RAM auf Erweiterungskarten o.Ä.) einfach keine Bitfields verwenden, sondern ganz normale uintXX_t .

Allein schon deswegen, weil memory-mapped Zugriffe meist recht langsam sind. Über PCI bzw. PCIe kann sowas - verglichen mit normalen RAM-Zugriffen - eine kleine Ewigkeit dauern.

Und ich vermute dass der von dir gezeigte CONTROL::operator= bei drei Bitfields zu drei Load-Modify-Store Sequenzen compilieren wird. Das ist erstmal langsamer als nötig, und dann kann es sogar sein dass es zu Fehlverhalten führt. z.B. wenn in einem Register ein "Start-/Stop-Bit" und weitere Steuerbits enthalten sind.

Und wenn du die einzelnen Bits unbedingt als Bitfields ansprechen möchtest, kannst du immer noch Load/Store Templates machen, die den passenden uintXX_t verwenden um auf den memory-mapped Bereich zuzugreifen, und dann memcpy bzw. ne char-Zeiger-Kopierschleife verwenden um die Daten aus bzw. in die Bitfield struct zu kopieren.

Beispiel:

#include <iostream>
#include <cstring>
#include <cstdint>

/////////////////////////////////////////////////////////////////////////////

template <class T, std::size_t N>
struct LoaderStorer;

template <class T, class U>
struct LoaderStorerImpl
{
	static T Load(void const* mem)
	{
		static_assert(sizeof(T) == sizeof(U), "size mismatch");
		U const u = *static_cast<U const volatile*>(mem);
		T t;
		std::memcpy(&t, &u, sizeof(T));
		return t;
	}

	static void Store(void* mem, T const& t)
	{
		static_assert(sizeof(T) == sizeof(U), "size mismatch");
		U u;
		std::memcpy(&u, &t, sizeof(T));
		*static_cast<U volatile*>(mem) = u;
	}
};

/////////////////////////////////////////////////////////////////////////////

template <class T>
struct LoaderStorer<T, sizeof(std::uint8_t)> : LoaderStorerImpl<T, std::uint8_t> {};

template <class T>
struct LoaderStorer<T, sizeof(std::uint16_t)> : LoaderStorerImpl<T, std::uint16_t> {};

template <class T>
struct LoaderStorer<T, sizeof(std::uint32_t)> : LoaderStorerImpl<T, std::uint32_t> {};

template <class T>
struct LoaderStorer<T, sizeof(std::uint64_t)> : LoaderStorerImpl<T, std::uint64_t> {};

/////////////////////////////////////////////////////////////////////////////

template <class T>
T Load(void const* mem)
{
	return LoaderStorer<T, sizeof(T)>::Load(mem);
}

template <class T>
void Store(void* mem, T const& t)
{
	LoaderStorer<T, sizeof(T)>::Store(mem, t);
}

/////////////////////////////////////////////////////////////////////////////

struct Foo
{
	uint32_t A : 2;
	uint32_t B : 3;
	uint32_t C : 4;
};

int main()
{
	char mem[1024];

	Foo f = {};
	f.A = 1;
	f.B = 2;
	f.C = 3;

	Store(&mem[32], f);

	Foo f2 = Load<Foo>(&mem[32]);
	f2.A++;
	f2.B++;
	f2.C++;
	Store(&mem[32], f2);

	Foo f3 = Load<Foo>(&mem[32]);

	std::cout << "A = " << f3.A << ", B = " << f3.B << ", C = " << f3.C << ".\n";
}

Zumindest MSVC ist hier schlau genug die memcpy Aufrufe komplett wegzuoptimieren. Die Load/Store Aufrufe werden inlined und zu einem einzigen mov reg, dword ptr [address] bzw. mov dword ptr [address], reg compiliert.
Und selbst wenn nicht, ist der Overhead vermutlich kleiner, als der von mehreren unnötigen memory-mapped Zugriffen.

[hustbaer]

ps:
Ich hab' gerade erst deinen ersten Beitrag gelesen - du wolltest dir ja eh im Prinzip sowas bauen. Nur halt mit der Adresse des Registers in eine Klasse eingewickelt. Mit den von mir gezeigten Load/Store Funktionen ist die Implementierung dann trivial:

// ... Load und Store definiert wie im obigen Beispiel ...

/////////////////////////////////////////////////////////////////////////////

template <typename T>
class Register
{
public:
	explicit Register(void* location)
		: m_location(location)
	{
	}

	T read() const
	{
		return Load<T>(m_location);
	}

	void write(T const& t)
	{
		Store(m_location, t);
	}

private:
	void* m_location;
};

/////////////////////////////////////////////////////////////////////////////

struct CONTROL {
	uint8_t
		enableTransmitter : 1,
		status : 4,
		: 1,
		crcMode : 2;
};

/////////////////////////////////////////////////////////////////////////////

int main(int argc, char* argv[])
{
	char hardwareMem[1024]; // Simuliertes mapped I/O memory 
	Register<CONTROL> r(hardwareMem);

	CONTROL c = {}; // Initialisieren würde ich das Ding schon...
	c.enableTransmitter = 1;
	c.crcMode = 0;

	r.write(c);

	// Folgende Zeilen WERDEN nicht wegoptimiert
	r.read();
	r.read();
	r.read();

	return 0;
}

Funktioniert bei mir wunderbar:

char hardwareMem[1024]; // Simuliertes mapped I/O memory 
	Register<CONTROL> r(hardwareMem);
00BB1013  lea         eax,[hardwareMem]  
00BB1019  mov         dword ptr [r],eax  

	CONTROL c = {}; // Initialisieren würde ich das Ding schon...
00BB101F  xor         al,al  

	CONTROL c = {}; // Initialisieren würde ich das Ding schon...
00BB1021  and         al,3Eh  
	c.enableTransmitter = 1;
	c.crcMode = 0;
00BB1023  or          al,1  
00BB1025  mov         byte ptr [c],al  

	r.write(c);
00BB102B  mov         byte ptr [hardwareMem],al  

	// Folgende Zeilen WERDEN nicht wegoptimiert
	r.read();
00BB1031  mov         al,byte ptr [hardwareMem]  
	r.read();
00BB1037  mov         al,byte ptr [hardwareMem]  
	r.read();
00BB103D  mov         al,byte ptr [hardwareMem]

Ein paar Dinge sind da komisch, z.B. die Variable c überhaupt im Speicher angelegt wird, bzw. das übermässig komplizierte rummachen mit al . Ich hätte mir erwartet dass jeder halbwegs brauchbare Optimizer es hinbekommt das wegzumachen. K.a. was MSVC sich dabei denkt.
Der Rest sieht aber ganz gut aus. Also alles bis auf die eigentlichen Speicherzugriffe wegoptimiert, kein Problem wegen der 3 Ebenen an Hilfsfunktionen, dem memcpy oder sonstwas.
Und das was bleiben muss ... ist geblieben

[hustbaer]

ps2:
GCC und Clang sind "etwas" schlauer:

mov     byte ptr [rsp - 8], 1
        mov     al, byte ptr [rsp - 8]
        mov     al, byte ptr [rsp - 8]
        mov     al, byte ptr [rsp - 8]
        xor     eax, eax
        ret

https://goo.gl/CuVzXa

[Finnegan]

hustbaer schrieb:

Und ich vermute dass der von dir gezeigte CONTROL::operator= bei drei Bitfields zu drei Load-Modify-Store Sequenzen compilieren wird.

Hi!

Hatte mir das grad auch nochmal angesehen. Noch nicht viel mit Bitfields gemacht, war aber eigentlich davon ausgegangen, dass er die Member alle wie ein einzelnes,
zusammenhängendes Byte behandelt. Du hast recht, der erzeugt tatsächlich Code um 3 mal hintereinander dasselbe Byte zu lesen und zu schreiben (VS2015, /Ox maximale Optimierung):

char mem[100];
    CONTROL c;
    volatile CONTROL& cv = *reinterpret_cast<CONTROL*>(mem);
    cv = c;

00007FF68FA41582  and         dl,0FEh  
00007FF68FA41585  movzx       ecx,al  
00007FF68FA41588  and         cl,1  
00007FF68FA4158B  or          dl,cl  
00007FF68FA4158D  movzx       ecx,al  
00007FF68FA41590  mov         byte ptr [mem],dl  
00007FF68FA41594  and         cl,1Eh  
00007FF68FA41597  movzx       edx,byte ptr [mem]  
00007FF68FA4159C  and         dl,0E1h  
00007FF68FA4159F  or          dl,cl  
00007FF68FA415A1  mov         byte ptr [mem],dl  
00007FF68FA415A5  movzx       ecx,byte ptr [mem]  
00007FF68FA415AA  xor         cl,al  
00007FF68FA415AC  and         cl,3Fh  
00007FF68FA415AF  xor         cl,al  
00007FF68FA415B1  mov         byte ptr [mem],cl

Wenn ich ein wenig drüber nachdenke, macht das sogar Sinn. Schliesslich könnte ich einer Hardware in einigen Bits eine Anfrage übermitteln,
deren Antwort ich dann postwendend in ein paar anderen Bits des selben Bytes erhalte.
Da die Zuweisungen separate Anweisungen sind, müssen sie sowas natürlich auch korrekt abbilden.

Also: Hustbaers Lösung gefällt mir auch deutlich besser als der ganze redundante Code oder das Makro-Geraffel.

Gruss,
Finnegan

[Stephan++ 0]

Wow, da habt ihr euch aber Mühe gegeben, lol. Danke!

Finnegan schrieb:

Ich habe allerdings trotzdem mal etwas mithilfe von Boost.Preprocessor zusammengestrickt

Das wäre ziemlich genau das, was ich suche. Nur leider darf ich in meiner Firma kein boost und keine stl benutzen, daher fällt diese Lösung leider flach.

Finnegan schrieb:

Frag mich aber nicht, wie Boost.Preprocessor das alles genau bewerkstelligt, ich habe da nur mal kurz reingeschaut wie man sowas umsetzt,
ich glaube das will man nicht wirklich alles wissen (ich zumindest nicht) - es sieht furchtbar aus .

Hihi, hat irgendwie was vom signal / slot System von Qt. Das ist auch schwarze Magie.

hustbaer schrieb:

Also ich würde bei memory-mapped Registern (bzw. auch RAM auf Erweiterungskarten o.Ä.) einfach keine Bitfields verwenden, sondern ganz normale uintXX_t.

Genau das möchte ich ja vermeiden. Mir geht es vor allem um die Lesbarkeit und Wartbarkeit des Codes. Und:

CONTROL.crc = 2;
CONTROL.enableTransmitter = 1;

Lässt sich hundert mal besser lesen als irgendwelche wilden binären Operationen wie:

int control = 0x02 | 0x04

Ich habe tatsächlich nur einen Bruchteil meiner Implementierung gezeigt. Die ist in der tat sehr ähnlich zur Lösung von hustbaer. Man kann sich quasi aussuchen, ob man direkt auf dem gememory gemappten bereich arbeiten will oder erst alles in einem temporärem Objekt zwischenspeichern und anschließend in einem Rutsch schreiben will (Bei manchen Registern muss man das sogar).
Und wie ihr bereits herausgefunden habt, optimiert der Compiler die Zugriffe in der Regel recht gut hin, daher sehe ich das mit den Performanceienbußen eher unkritisch. Ich finde die Vorteile der besseren Lesbarkeit und Wartbarkeit überwiegen hier.

Mit der Klasse arbeite ich bereits eine weile und möchte sie eigentlich nicht mehr missen. Einzig das Problem mit dem volatile ist noch etwas nervig. Zwar scheint meine dreckige cast Lösung erst einmal zu funktionieren, aber sobald meine Firma den compiler wechselt stehe ich eventuell dumm da. Notfalls führe ich eine explizite dummyRead() funktion ein, die dann einfach über einen volatile int Zeiger das Register liest. Dann müsste ich auch keine Zuweisungsoperatoren mehr überladen und der Code wäre an stellen, wo dummy reads gefordert sind, zugleich selbsterklärender.

Gruß
Stephan

[sebi707]

Stephan++ schrieb:

hustbaer schrieb:

Also ich würde bei memory-mapped Registern (bzw. auch RAM auf Erweiterungskarten o.Ä.) einfach keine Bitfields verwenden, sondern ganz normale uintXX_t.

Genau das möchte ich ja vermeiden. Mir geht es vor allem um die Lesbarkeit und Wartbarkeit des Codes.

Leider ist auch so ziemlich alles was Bitfields angeht Implementation defined. Ob du also das von dir gewünsche Speicherlayout kriegst ist also keinesfalls garantiert.

[hustbaer]

Stephan++ schrieb:

CONTROL.crc = 2;
CONTROL.enableTransmitter = 1;

Lässt sich hundert mal besser lesen als irgendwelche wilden binären Operationen wie:

int control = 0x02 | 0x04

Niemand hast gesagt dass du Magic-Numbers im Code verstreuen sollst.
Man munkelt dass man in C++ auch sowas wie Konstanten definieren kann. Google mal, vielleicht findest du nen Artikel wo das beschrieben wird.

[hustbaer]

Stephan++ schrieb:

Ich habe tatsächlich nur einen Bruchteil meiner Implementierung gezeigt. Die ist in der tat sehr ähnlich zur Lösung von hustbaer. Man kann sich quasi aussuchen (...)

OK. Schön dass du eine so praktische toll funktionierende Lösung hast.

Nur... was soll dann die Frage und "es compiliert einfach nicht *wein* *wein*"? Willst du uns bloss verarschen?

[Stephan++ 0]

Verarschen? Es geht mir von Anfang an um das volatile Problem. Ich habe nie danach gefragt, ob mir jemand eine komplette Lösung vorkauen will. Mit dem reduzierten Code habe ich mich lediglich an die Forenregeln gehalten... Wie mans macht, macht mans falsch.

[hustbaer]

OK. Whatever.

Gibt es jetzt noch ein für dich ungelöstes Problem, oder hast du alles was du brauchst?
Ich werde nämlich aus deinem Beitrag so überhaupt nicht schlau.