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

== 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.

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

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.

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.

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.

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).

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.

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.

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