1797933 - wasm-gc via Ion: implement struct.{new,set,get} and ref.cast

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement)

Description

[Julian Seward [:jseward]]

In other words, implement enough so that js/src/jit-test/tests/wasm/gc/structs.js
will run with --wasm-compiler=ion.

Comment 1

[Julian Seward [:jseward]]

Attachment: Bug 1797933, part 1 - rename {M,L}WasmLoadObjectField et al. r=rhunt.

Naming of MIR and LIR nodes WasmLoadObjectField et al is somewhat confusing:

• It is inconsistent which of the fields point to the base of the memory area
  to access, and which if any are the keepalive-object pointer, eg:

  • WasmLoadObjectField: obj is the base pointer
  • WasmLoadObjectDataField: data is the base pointer, obj is the keepalive

• having Data in the name doesn't really convey the meaning "also carries a
  keepalive pointer"

They are renamed thusly:

WasmLoadObjectField -> WasmLoadObjectField (unchanged)
WasmLoadObjectDataField -> WasmLoadObjectFieldWithKA
WasmStoreObjectDataField -> WasmStoreObjectFieldWithKA
WasmStoreObjectDataRefField -> WasmStoreObjectRefFieldWithKA

Changes:

• change Data in the name to WithKA at the end
• consistently use ka as the name of the keepalive MIR/LIR node
• consistently use obj as the name of the base pointer MIR/LIR node

It could be argued that the "Object" should be removed from the name too,
since these don't actually care about or have any knowledge of the fact that
they are loads/stores from some offset inside an object. They do "care" about
the fact that they are JS heap accesses though. But the above changes are
enough for now.

Comment 2

[Julian Seward [:jseward]]

Attachment: Bug 1797933, part 2 - initial implementation of struct.{new,set,get} and ref.cast for Ion. r=rhunt.

This patch implements struct.{new,set,get} and ref.cast for Ion. The
implementation is in principle straightforward and is derived from the
baseline equivalents. There are however many pieces that need to be
coordinated. Changes:

• Compiler gating logic -- enable Ion for wasm GC extensions.

• Pertaining to the above, extend the guard conditions in many of the wasm/gc
  test cases so as to limit them to baseline only, for now, so that we don't
  get failures as a result of gc insns that are currently unimplemented in
  Ion. This will need to be incrementally undone as more gc insns get
  implemented in Ion.

• Pertaining to testing (lib/wasm-binary.js), remove wasm gc opcodes from the
  lists of opcodes that we check are unimplemented.

• New test file wasm/gc/structs2.js, that tests 8- and 16-bit field accesses,
  since the existing structs.js file doesn't.

• Add support to MIR and LIR to handle 8- and 16-bit memory accesses. One
  possibility would have been to add MIRType::Int16 and MIRType::Int8, but I
  didn't want to destabilise the existing, probably carefully-balanced MIR
  optimisation machinery for JS. So I chose to add auxiliary descriptors just
  for the 4 relevant MIR/LIR nodes instead:

  • New enums MNarrowingOp and MWideningOp, to describe how to narrow to /
    widen from Int32.

  • Renamed wasm::FieldExtension to wasm::FieldWideningOp for consistency with
    the above. Note MWideningOp and wasm::FieldWideningOp both need to exist;
    neither can exactly do the job of the other.

  • LIRGenerator::visitWasmLoadObjectField/LoadObjectFieldWithKA
    /StoreObjectFieldWithKA: handle 8- and 16-bit accesses.
    WasmStoreObjectRefFieldWithKA is unaffected since a ref can't be 8- or
    16-bits long.

  • CodeGenerator::visitWasm{Store,Load}Slot: handle 8- and 16-bit accesses.

• The actual implementation of struct.{new,set,get} and ref.cast, which is
  pretty straightforward:

  • new helper function FunctionCompiler::loadGcCanon

  • new helper function WriteValueToWasmObjectField, used for both struct.new
    and struct.set.

  • new helpers FieldDescriptorToMIREquivalents and FieldTypeToMNarrowingOp,
    used to create the right MIR descriptions for 8- and 16-bit transactions.

• Ridealong fix: FunctionCompiler::loadExceptionValues: add missing OOM check
  for auto* data = ...

Depends on D161252

Comment 3

[Julian Seward [:jseward]]

Attachment: Bug 1797933, part 3 - add alias-set annotations for struct.{new,set,get} code. r=rhunt.

This patch adds alias set annotations to the MIR generated for
struct.{new,set,get}, so as to enable the existing GVN machinery to remove
duplicate loads of the OOL block pointer in the (static) presence of multiple
OOL field accesses to the same wasm object.

This is a bit tricky because we must ensure that neither an IL-data-field nor
OOL-data-field write to the object invalidate the OOL-block-pointer read.
Hence the OOL-block-pointer-field cannot be in the same alias set as either
the IL- nor OOL-data fields. And so this patch adds three new alias-set
descriptors.

The implementation is straightforward and described in comments.

Because it is easy to mess up optimisation with incorrect alias set
descriptors, the MWasm{Load,Store}ObjectField*::New methods heavily restrict
what descriptors they accept, via assertions.

Because those same MIR nodes are also used to implement exceptions, they also
accept AliasSet::{Load,Store}(AliasSet::All) ("no information") descriptors.
The exception-handling MIR is unaffected by this patch.

Depends on D161253

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5eb1fdf9665
part 1 - rename {M,L}WasmLoadObjectField et al.  r=rhunt.

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/a5eb1fdf9665

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f0dec5c5816
part 2 - initial implementation of struct.{new,set,get} and ref.cast for Ion.  r=rhunt.
https://hg.mozilla.org/integration/autoland/rev/02cc7ae51df3
part 3 - add alias-set annotations for struct.{new,set,get} code.  r=rhunt.

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9f0dec5c5816
https://hg.mozilla.org/mozilla-central/rev/02cc7ae51df3