Closed Bug 1797933 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: JavaScript: WebAssembly, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
109 Branch
Tracking Status
firefox109 --- fixed

People

(Reporter: jseward, Assigned: jseward)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

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

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.

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

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

Blocks: 1799856
Depends on: 1801108
Attachment #9301952 - Attachment description: Bug 1797933, part 2 - initial implementation struct.{new,set,get} and ref.cast for Ion. r=rhunt. → Bug 1797933, part 2 - initial implementation of struct.{new,set,get} and ref.cast for Ion. r=rhunt.
Pushed by jseward@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a5eb1fdf9665 part 1 - rename {M,L}WasmLoadObjectField et al. r=rhunt.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch
Pushed by jseward@mozilla.com: 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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: