www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Possible bug in atomicOp

reply Benjamin Thaut <code benjamin-thaut.de> writes:
The following testcase (when executed on dual core at least) results in 
a endless loop inside atomicOp.

import std.stdio;
import std.concurrency;

enum Messages {
	GO,
	END
}

shared class Account {
	private double amount = 0;
	
	double getAmount() const {
		return amount;
	}
	
	void change(double change){
		atomicOp!"+="(amount,change);
	}
}

shared Account bank = null;
	
void otherThread(Tid father){
	send(father,Messages.GO);
	for(int i=0;i<1000;i++)
		bank.change(-100);
	send(father,Messages.END);
}

void main(string[] args)
{
	bank = new Account();
	spawn(&otherThread,thisTid);
	receiveOnly!(Messages)();
	for(int i=0;i<1000;i++)
		bank.change(+100);
	receiveOnly!(Messages)();
	writefln("Program finished. Amount is %s",bank.getAmount());
}

Is this a bug, or am I doing something wrong here?
If it is a bug it is kind of critical because people which are reading 
the "the D programming language" book won't be happy to find out that 
some of the given examples do not work yet.
-- 
Kind Regards
Benjamin Thaut
Oct 23 2010
parent reply dsimcha <dsimcha yahoo.com> writes:
== Quote from Benjamin Thaut (code benjamin-thaut.de)'s article

 The following testcase (when executed on dual core at least) results in
 a endless loop inside atomicOp.
 import std.stdio;
 import std.concurrency;
 enum Messages {
 	GO,
 	END
 }
 shared class Account {
 	private double amount = 0;
 	double getAmount() const {
 		return amount;
 	}
 	void change(double change){
 		atomicOp!"+="(amount,change);
 	}
 }
 shared Account bank = null;
 void otherThread(Tid father){
 	send(father,Messages.GO);
 	for(int i=0;i<1000;i++)
 		bank.change(-100);
 	send(father,Messages.END);
 }
 void main(string[] args)
 {
 	bank = new Account();
 	spawn(&otherThread,thisTid);
 	receiveOnly!(Messages)();
 	for(int i=0;i<1000;i++)
 		bank.change(+100);
 	receiveOnly!(Messages)();
 	writefln("Program finished. Amount is %s",bank.getAmount());
 }
 Is this a bug, or am I doing something wrong here?
 If it is a bug it is kind of critical because people which are reading
 the "the D programming language" book won't be happy to find out that
 some of the given examples do not work yet.


http://d.puremagic.com/issues/show_bug.cgi?id=4782

Basically, atomicLoad (which atomicOp uses) always returns in ALU registers.
Floating point numbers need to be returned in floating point registers.
Therefore, a NaN always gets returned from atomicLoad!double, and a NaN isn't
equal to anything.
Oct 23 2010
parent reply Benjamin Thaut <code benjamin-thaut.de> writes:
Am 23.10.2010 14:52, schrieb dsimcha:

 == Quote from Benjamin Thaut (code benjamin-thaut.de)'s article

 The following testcase (when executed on dual core at least) results in
 a endless loop inside atomicOp.
 import std.stdio;
 import std.concurrency;
 enum Messages {
 	GO,
 	END
 }
 shared class Account {
 	private double amount = 0;
 	double getAmount() const {
 		return amount;
 	}
 	void change(double change){
 		atomicOp!"+="(amount,change);
 	}
 }
 shared Account bank = null;
 void otherThread(Tid father){
 	send(father,Messages.GO);
 	for(int i=0;i<1000;i++)
 		bank.change(-100);
 	send(father,Messages.END);
 }
 void main(string[] args)
 {
 	bank = new Account();
 	spawn(&otherThread,thisTid);
 	receiveOnly!(Messages)();
 	for(int i=0;i<1000;i++)
 		bank.change(+100);
 	receiveOnly!(Messages)();
 	writefln("Program finished. Amount is %s",bank.getAmount());
 }
 Is this a bug, or am I doing something wrong here?
 If it is a bug it is kind of critical because people which are reading
 the "the D programming language" book won't be happy to find out that
 some of the given examples do not work yet.


 http://d.puremagic.com/issues/show_bug.cgi?id=4782

 Basically, atomicLoad (which atomicOp uses) always returns in ALU registers.
 Floating point numbers need to be returned in floating point registers.
 Therefore, a NaN always gets returned from atomicLoad!double, and a NaN isn't
 equal to anything.


So shouldn't there be a static assert to prevent one from using atomicOp 
with floats and doubles? Or should atomicLoad be implemented to support 
floats and doubles?

-- 
Kind Regards
Benjamin Thaut
Oct 23 2010
parent reply Sean Kelly <sean invisibleduck.org> writes:
Benjamin Thaut <code benjamin-thaut.de> wrote:

 Am 23.10.2010 14:52, schrieb dsimcha:

 == Quote from Benjamin Thaut (code benjamin-thaut.de)'s article

 The following testcase (when executed on dual core at least) results
 in
 a endless loop inside atomicOp.
 import std.stdio;
 import std.concurrency;
 enum Messages {
 	GO,
 	END
 }
 shared class Account {
 	private double amount = 0;
 	double getAmount() const {
 		return amount;
 	}
 	void change(double change){
 		atomicOp!"+="(amount,change);
 	}
 }
 shared Account bank = null;
 void otherThread(Tid father){
 	send(father,Messages.GO);
 	for(int i=0;i<1000;i++)
 		bank.change(-100);
 	send(father,Messages.END);
 }
 void main(string[] args)
 {
 	bank = new Account();
 	spawn(&otherThread,thisTid);
 	receiveOnly!(Messages)();
 	for(int i=0;i<1000;i++)
 		bank.change(+100);
 	receiveOnly!(Messages)();
 	writefln("Program finished. Amount is %s",bank.getAmount());
 }
 Is this a bug, or am I doing something wrong here?
 If it is a bug it is kind of critical because people which are
 reading
 the "the D programming language" book won't be happy to find out
 that
 some of the given examples do not work yet.


 
 http://d.puremagic.com/issues/show_bug.cgi?id=4782
 
 Basically, atomicLoad (which atomicOp uses) always returns in ALU
 registers.
 Floating point numbers need to be returned in floating point
 registers.
 Therefore, a NaN always gets returned from atomicLoad!double, and a
 NaN isn't
 equal to anything.


 
 So shouldn't there be a static assert to prevent one from using
 atomicOp with floats and doubles? Or should atomicLoad be implemented
 to support floats and doubles?


The former in the short term and the latter in the long term.
Oct 23 2010
parent reply "Robert Jacques" <sandford jhu.edu> writes:
On Sat, 23 Oct 2010 15:50:30 -0400, Sean Kelly <sean invisibleduck.org>  
wrote:


 Benjamin Thaut <code benjamin-thaut.de> wrote:

 Am 23.10.2010 14:52, schrieb dsimcha:

 == Quote from Benjamin Thaut (code benjamin-thaut.de)'s article

 The following testcase (when executed on dual core at least) results
 in
 a endless loop inside atomicOp.
 import std.stdio;
 import std.concurrency;
 enum Messages {
 	GO,
 	END
 }
 shared class Account {
 	private double amount = 0;
 	double getAmount() const {
 		return amount;
 	}
 	void change(double change){
 		atomicOp!"+="(amount,change);
 	}
 }
 shared Account bank = null;
 void otherThread(Tid father){
 	send(father,Messages.GO);
 	for(int i=0;i<1000;i++)
 		bank.change(-100);
 	send(father,Messages.END);
 }
 void main(string[] args)
 {
 	bank = new Account();
 	spawn(&otherThread,thisTid);
 	receiveOnly!(Messages)();
 	for(int i=0;i<1000;i++)
 		bank.change(+100);
 	receiveOnly!(Messages)();
 	writefln("Program finished. Amount is %s",bank.getAmount());
 }
 Is this a bug, or am I doing something wrong here?
 If it is a bug it is kind of critical because people which are
 reading
 the "the D programming language" book won't be happy to find out
 that
 some of the given examples do not work yet.


 http://d.puremagic.com/issues/show_bug.cgi?id=4782

 Basically, atomicLoad (which atomicOp uses) always returns in ALU
 registers.
 Floating point numbers need to be returned in floating point
 registers.
 Therefore, a NaN always gets returned from atomicLoad!double, and a
 NaN isn't
 equal to anything.


 So shouldn't there be a static assert to prevent one from using
 atomicOp with floats and doubles? Or should atomicLoad be implemented
 to support floats and doubles?


 The former in the short term and the latter in the long term.


Well, here's the assembler to load a value onto the FP stack:
float  __int2float  (ref int  x) { asm { fld float  ptr [EAX]; } }
double __long2double(ref long x) { asm { fld double ptr [EAX]; } }

Unfortunately, direct loading from a register doesn't seem to be  
supported. So writing wrapper code which uses a union would be almost as  
fast. I know there is an SSE instruction to load directly from registers,  
but we can't assume SSE support.
Oct 24 2010
parent reply Don <nospam nospam.com> writes:
Robert Jacques wrote:

 On Sat, 23 Oct 2010 15:50:30 -0400, Sean Kelly <sean invisibleduck.org> 
 wrote:

 Basically, atomicLoad (which atomicOp uses) always returns in ALU
 registers.
 Floating point numbers need to be returned in floating point
 registers.
 Therefore, a NaN always gets returned from atomicLoad!double, and a
 NaN isn't
 equal to anything.


 So shouldn't there be a static assert to prevent one from using
 atomicOp with floats and doubles? Or should atomicLoad be implemented
 to support floats and doubles?


 The former in the short term and the latter in the long term.


 
 Well, here's the assembler to load a value onto the FP stack:
 float  __int2float  (ref int  x) { asm { fld float  ptr [EAX]; } }
 double __long2double(ref long x) { asm { fld double ptr [EAX]; } }


That should be:
fild dword ptr [EAX];
fild qword ptr [EAX];


 Unfortunately, direct loading from a register doesn't seem to be 
 supported. So writing wrapper code which uses a union would be almost as 
 fast. I know there is an SSE instruction to load directly from 
 registers, but we can't assume SSE support.
Oct 24 2010
next sibling parent reply Don <nospam nospam.com> writes:
Don wrote:

 Robert Jacques wrote:

 On Sat, 23 Oct 2010 15:50:30 -0400, Sean Kelly 
 <sean invisibleduck.org> wrote:

 Basically, atomicLoad (which atomicOp uses) always returns in ALU
 registers.
 Floating point numbers need to be returned in floating point
 registers.
 Therefore, a NaN always gets returned from atomicLoad!double, and a
 NaN isn't
 equal to anything.


 So shouldn't there be a static assert to prevent one from using
 atomicOp with floats and doubles? Or should atomicLoad be implemented
 to support floats and doubles?


 The former in the short term and the latter in the long term.


 Well, here's the assembler to load a value onto the FP stack:
 float  __int2float  (ref int  x) { asm { fld float  ptr [EAX]; } }
 double __long2double(ref long x) { asm { fld double ptr [EAX]; } }


 
 That should be:
 fild dword ptr [EAX];
 fild qword ptr [EAX];
 

 Unfortunately, direct loading from a register doesn't seem to be 
 supported. So writing wrapper code which uses a union would be almost 
 as fast. I know there is an SSE instruction to load directly from 
 registers, but we can't assume SSE support.




Wait a minute. x86 has no read-modify-write instructions for x87, or for 
SSE. So I don't think it's possible to implement atomic floating-point 
ops, apart from assignment.
(Of course, you can fake it with a mass of casts and a CAS, but that 
doesn't seem helpful).
It should just be prevented, except possibly for assignment. (Is an 
80-bit float assignment atomic? Maybe not, since it would need special 
logic).
Oct 25 2010
parent reply Sean Kelly <sean invisibleduck.org> writes:
Don Wrote:

 
 Wait a minute. x86 has no read-modify-write instructions for x87, or for 
 SSE. So I don't think it's possible to implement atomic floating-point 
 ops, apart from assignment.
 (Of course, you can fake it with a mass of casts and a CAS, but that 
 doesn't seem helpful).
 It should just be prevented, except possibly for assignment. (Is an 
 80-bit float assignment atomic? Maybe not, since it would need special 
 logic).


atomicOp does most of its RMW operations using a CAS loop, so I think it should
work.  The redo occurs when the memory location being written to changed since
it was read, and that shouldn't be any different for floating point values vs.
integers.
Oct 25 2010
next sibling parent Fawzi Mohamed <fawzi gmx.ch> writes:
On 26-ott-10, at 05:13, Sean Kelly wrote:


 Don Wrote:

 Wait a minute. x86 has no read-modify-write instructions for x87,  
 or for
 SSE. So I don't think it's possible to implement atomic floating- 
 point
 ops, apart from assignment.
 (Of course, you can fake it with a mass of casts and a CAS, but that
 doesn't seem helpful).
 It should just be prevented, except possibly for assignment. (Is an
 80-bit float assignment atomic? Maybe not, since it would need  
 special
 logic).


 atomicOp does most of its RMW operations using a CAS loop, so I  
 think it should work.  The redo occurs when the memory location  
 being written to changed since it was read, and that shouldn't be  
 any different for floating point values vs. integers.



I use atomic op casting pointers as integer pointers (
http://github.com/fawzi/blip/blob/master/blip/sync/Atomic.d 
  which is also tango one), and I haven't encountered any problem yet,  
but I haven't checked in detail if some x87 or SSE load/store might  
potentially give problems, but as sean says they should not (as the  
value should be transferred from register to register, not going  
through the memory, and memory accesses are controlled by CAS, and  
memory barriers (if used to separate subsequent "normal" ops) should  
be valid also for x87/SEE.

Fawzi
Oct 26 2010
prev sibling parent Sean Kelly <sean invisibleduck.org> writes:
Sean Kelly Wrote:


 Don Wrote:

 
 Wait a minute. x86 has no read-modify-write instructions for x87, or for 
 SSE. So I don't think it's possible to implement atomic floating-point 
 ops, apart from assignment.
 (Of course, you can fake it with a mass of casts and a CAS, but that 
 doesn't seem helpful).
 It should just be prevented, except possibly for assignment. (Is an 
 80-bit float assignment atomic? Maybe not, since it would need special 
 logic).


 
 atomicOp does most of its RMW operations using a CAS loop, so I think it
should work.  The redo occurs when the memory location being written to changed
since it was read, and that shouldn't be any different for floating point
values vs. integers.


ref: http://dsource.org/projects/druntime/browser/trunk/src/core/atomic.d#L127
Oct 26 2010
prev sibling parent "Robert Jacques" <sandford jhu.edu> writes:
On Mon, 25 Oct 2010 00:08:03 -0400, Don <nospam nospam.com> wrote:


 Robert Jacques wrote:

 On Sat, 23 Oct 2010 15:50:30 -0400, Sean Kelly <sean invisibleduck.org>  
 wrote:

 Basically, atomicLoad (which atomicOp uses) always returns in ALU
 registers.
 Floating point numbers need to be returned in floating point
 registers.
 Therefore, a NaN always gets returned from atomicLoad!double, and a
 NaN isn't
 equal to anything.


 So shouldn't there be a static assert to prevent one from using
 atomicOp with floats and doubles? Or should atomicLoad be implemented
 to support floats and doubles?


 The former in the short term and the latter in the long term.


  Well, here's the assembler to load a value onto the FP stack:
 float  __int2float  (ref int  x) { asm { fld float  ptr [EAX]; } }
 double __long2double(ref long x) { asm { fld double ptr [EAX]; } }


 That should be:
 fild dword ptr [EAX];
 fild qword ptr [EAX];


Opps. I should have named them __int_as_float and __long_as_double. The  
point was to find an efficient way to return an int or double in register  
on the x87 stack.
Oct 25 2010