Closed Bug 1131624 Opened 5 years ago Closed 5 years ago

Support float64 atomics

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1131613

People

(Reporter: lth, Unassigned)

References

(Blocks 1 open bug)

Details

On most platforms we can support float64 atomics efficiently using CAS or LL/SC; only ARMv6 and MIPS32 are known to not have the necessary hardware support among the platforms we care about.

Thus we should support efficient float64 atomics on the nice platforms, and float64 atomics by means of a spinlock (on the SAB) or similar on the less-nice platforms.

Requires updates to the VM, the JITs, and the spec.

One hard part is that the VM - really the C++ compiler - and the JIT must agree on the mechanism being used for mutual exclusion.  (That's already true but has caused no problems so far, or none that I know about.)  If the JIT will use an 8-byte CAS then the C++ code must do so too; if the JIT will use the spinlock then the C++ code must do so too.
See notes on complications: https://bugzilla.mozilla.org/show_bug.cgi?id=1131613#c2
See Also: → 1131613
I've implemented 64-bit atomic loads and stores on doubles in Emscripten as a spin loop emulation:

https://github.com/juj/emscripten-fastcomp/commit/150c6c74115e6db353f2a0d2cf1fa5e96816c8e6

and

https://github.com/juj/emscripten/commit/69c311196495a77719848aea5afadf6b99af3c2e

The ops were implemented off the top of my head and I haven't put too much thought into them, as I currently don't have any code that would require it, and I don't have a test written up yet, however, if you have a second, some auditing on that code path will not hurt :)

Note that it would be very nice to have 64-bit integer atomic load, store and CAS ops, even though JS doesn't yet have a 64-bit type. Perhaps that API could take in the high and low 32-bit parts of the number (I know some people might find that super-ugly, but can't think of anything better). I am sure that we will run into C/C++ code in the wild that will use 64-bit atomic ops pretty easily. 64bit int atomic ops are probably even more important than 64bit float atomic ops.
(In reply to Jukka Jylänki from comment #2)
> I've implemented 64-bit atomic loads and stores on doubles in Emscripten as
> a spin loop emulation:
> 
> https://github.com/juj/emscripten-fastcomp/commit/
> 150c6c74115e6db353f2a0d2cf1fa5e96816c8e6
> 
> and
> 
> https://github.com/juj/emscripten/commit/
> 69c311196495a77719848aea5afadf6b99af3c2e
> 
> The ops were implemented off the top of my head and I haven't put too much
> thought into them, as I currently don't have any code that would require it,
> and I don't have a test written up yet, however, if you have a second, some
> auditing on that code path will not hurt :)

I don't think these are right, but I need to spend more time looking at them, I could be missing something.  Also see below.

> Note that it would be very nice to have 64-bit integer atomic load, store
> and CAS ops, even though JS doesn't yet have a 64-bit type. Perhaps that API
> could take in the high and low 32-bit parts of the number (I know some
> people might find that super-ugly, but can't think of anything better). I am
> sure that we will run into C/C++ code in the wild that will use 64-bit
> atomic ops pretty easily. 64bit int atomic ops are probably even more
> important than 64bit float atomic ops.

I was just thinking about this too, but returning multiple values is a pain.  I'll think about it some more.

A "known correct" way to do 64-bit atomic ops at the moment is to use an additional word as a spinlock.  Atomic operations must acquire the lock, load/store/whatever the value, and release the lock again.  A simple spinlock requires one additional location (usually a word is right).  The location could be global for simplicity, though with poor performance.  For float64 the operations could look like this:

https://github.com/lars-t-hansen/parlib-simple/blob/master/src/float64atomics.js

Although those definitions use numeric equality, not bitwise equality, for CAS.
(In reply to Lars T Hansen [:lth] from comment #3)
> (In reply to Jukka Jylänki from comment #2)
> > I've implemented 64-bit atomic loads and stores on doubles in Emscripten as
> > a spin loop emulation:
> > 
> > https://github.com/juj/emscripten-fastcomp/commit/
> > 150c6c74115e6db353f2a0d2cf1fa5e96816c8e6
> > 
> > and
> > 
> > https://github.com/juj/emscripten/commit/
> > 69c311196495a77719848aea5afadf6b99af3c2e
> > 
> > The ops were implemented off the top of my head and I haven't put too much
> > thought into them, as I currently don't have any code that would require it,
> > and I don't have a test written up yet, however, if you have a second, some
> > auditing on that code path will not hurt :)
> 
> I don't think these are right, but I need to spend more time looking at
> them, I could be missing something.  Also see below.

Yeah, I am a bit skeptical myself, but chose not to spend more time until I have the right moment to write up a stress test. Thanks for the link on the spinlock. I was wondering for a moment to do e.g. a global array of 16, 32 or 64 such memory locations for global spinlocks, and take the address modulo the number of these spinlocks to choose which word to lock on. This might give a good probability that the locks will always occur on separate addresses, and therefore the chance of contention would be low.
(In reply to Jukka Jylänki from comment #4)

> I was wondering for a moment to do e.g. a global array of 16, 32
> or 64 such memory locations for global spinlocks, and take the address
> modulo the number of these spinlocks to choose which word to lock on.

That's one possible optimizations, there are some notes on this in the spec document in the section "General guidance", which discusses all of this in more detail.  (Also I guess simple spinlocks have terrible performance and risk starvation.)
Also see https://bugzilla.mozilla.org/show_bug.cgi?id=1131613#c8, which discusses the need to canonicalize any NaN values following CAS or LOAD.
This work is folded into bug 1131613, as a consequence of how the code is coming together.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1131613
You need to log in before you can comment on or make changes to this bug.