1205390 - asm.js: Make the move from SharedTypedArray to TypedArray

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Lars T Hansen [:lth]]

It's easy to make Odin accept the use of TypedArray constructors even for shared memory, thus bringing Odin closer to spec without having to land all the changes to remove SharedTypedArray.

The benefit of landing this early is that emscripten can move toward spec compliance sooner, and other implementations and Firefox will be able to share asm.js code.

Comment 1

[Lars T Hansen [:lth]]

Attachment: bug1205390-ta-on-shmem.patch

POC, passes some basic testing.  Includes some test cases that need further work.

Comment 2

[Lars T Hansen [:lth]]

Attachment: Some further test cases

Comment 3

[Lars T Hansen [:lth]]

In the case of emscripten (or asm.js generally), the stdlib that is passed to the module constructor should not be "this" but should instead be an object that looks roughly as follows:

const atomicStdlib = {
    Atomics:      Atomics,
    Int8Array:    this.SharedInt8Array ? SharedInt8Array : Int8Array,
    Uint8Array:   this.SharedUint8Array ? SharedUint8Array : Uint8Array,
    Int16Array:   this.SharedInt16Array ? SharedInt16Array : Int16Array,
    Uint16Array:  this.SharedUint16Array ? SharedUint16Array : Uint16Array,
    Int32Array:   this.SharedInt32Array ? SharedInt32Array : Int32Array,
    Uint32Array:  this.SharedUint32Array ? SharedUint32Array : Uint32Array,
    Float32Array: this.SharedFloat32Array ? SharedFloat32Array : Float32Array,
    Float64Array: this.SharedFloat64Array ? SharedFloat64Array : Float64Array,
    ...
};

where ... indicates other things you might need.  This construction will allow the code to run in Firefox and non-Firefox browsers, and in Firefox even with --no-asmjs or a similar switch.

This hack can go away when bug 1176214 lands, but it will continue to work even then (Firefox will appear to the above code as a "non-Firefox" browser).

Comment 4

[Lars T Hansen [:lth]]

Attachment: bug1205390-ta-on-shmem.patch

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 8677606 [details] [diff] [review]
bug1205390-ta-on-shmem.patch

Review of attachment 8677606 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/asmjs/AsmJSModule.h
@@ +1112,5 @@
>      }
> +    void setViewsAreShared() {
> +        if (pod.hasArrayView_)
> +            pod.isSharedView_ = true;
> +        for ( size_t i=0 ; i < globals_.length() ; i++ ) {

Can you remove the spaces in "( size_t" and "i++ )"?

::: js/src/asmjs/AsmJSValidate.cpp
@@ +1516,5 @@
>      }
>  
>      void startFunctionBodies() {
> +        if (atomicsPresent_) {
> +            for ( GlobalMap::Range r = globals_.all() ; !r.empty() ; r.popFront() ) {

Can you remove spaces in "( GlobalMap" and "popFront() )"?

Comment 6

[Lars T Hansen [:lth]]

Filed emscripten bug to surface this feature: https://github.com/kripken/emscripten/issues/3869.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8f72e3889e80

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/259d7b810268

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8f72e3889e80
https://hg.mozilla.org/mozilla-central/rev/259d7b810268