Closed Bug 1784151 Opened 2 years ago Closed 2 years ago

Implement core array instructions in Ion

Categories

(Core :: JavaScript: WebAssembly, task, P3)

task

Tracking

()

RESOLVED WONTFIX

People

(Reporter: rhunt, Unassigned)

References

Details

Implement the core array instructions in Ion:

array.new
array.new_default
array.get
array.get_s
array.get_u
array.set
array.len

This excludes new_fixed/new_elem/new_data, which will be a separate bug.

These (or the struct) instructions will be the first GC proposal instructions in Ion. There are several MIR instructions added for exception-handling that we should re-use. A WebAssembly.Exception object has a data_ pointer that stores the fields of the thrown exception, very similar to how OutlineTypedObject works.

  1. MWasmLoadObjectField - loads a field directly off the GC object (such as anything in an InlineTypedObject, or the data_ pointer on an OutlineTypedObject)
  2. MWasmLoadObjectDataField - loads a field off of a data_ pointer of a GC object
  3. MWasmStoreObjectDataField - stores a non-reference-type field to the data_ pointer of a GC object
  4. MWasmStoreObjectDataRefField - stores a reference-typed field to the data_ pointer of a GC object

All of the instructions with 'Data' in their name are meant to be used on the data_ pointer. The reason they're separate instructions is that they take the original GC object as an extra parameter in order to keep it alive while the data pointer is accessed.

An example of using these instructions [1].

I think for arrays, the complication is that there is a dynamic index with bounds check. The LoadObject/StoreObject instructions above assume a constant index. Two options to look into:

(1) generalizing the instructions to take any MDefinition as the offset which could be an MConstant and specializing codegen to fold the constant into the addressing mode when it can.
(2) duplicating the instructions for a dynamic index version

There's a lot of flexibility in Ion, we could share the same MIR for structs/arrays but specialize to different LIR.

As for bounds checks, we should use MWasmBoundsCheck and add a new MWasmBoundsCheck::Target::Array. The new target should ensure that WasmBCE.cpp won't remove these bounds checks yet, which is what we want.

[1] https://searchfox.org/mozilla-central/rev/6ec440e105c2b75d5cae9d34f957a2f85a106d54/js/src/wasm/WasmIonCompile.cpp#3173

This work was done in bug 1797933, bug 1799856, and bug 1803043.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.