1783956 - Add MWasmTrapIfNull/Zero to simplify MIR

Status: RESOLVED FIXED

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

Description

[Yury Delendik (:yury)]

Per conversation at https://phabricator.services.mozilla.com/D154146#inline-848543 :

we should add a MWasmNullCheck instruction which takes a MIRType::RefOrNull operand and performs the null check/branch/trap in a single instruction. This will reduce the number of blocks we create, which will reduce the work that regalloc needs to do.

This shall improve the performance of Wasm function-reference and GC code/instructions.

Comment 1

[Yury Delendik (:yury)]

Attachment: Bug 1783956 - Add MWasmNullCheck to improve the performance of Ion. r?jseward

• Refactor checkNull() to create MWasmNullCheck instead of null constant and comparison.
• Implement LIsWasmNullAndBranch to do pointer test and branching

Comment 2

[Julian Seward [:jseward]]

Attachment: bug1783956-mwasm-null-check-musings.diff-1

Proof-of-concept patch to for generating the required code without adding any new MIR or LIR node kinds. Suggested usage:

• apply patch from bug 1741395 comment 4
• add this patch

IONFLAGS=codegen,dump-mir-expr,mir-to-lir ./dist/bin/js --no-ion --wasm-compiler=ion
--wasm-function-references --no-threads testcase.js

This should generate identical code for the two fns in testcase.js, on x86_64-linux. (one is currently commented out)

Comment 3

[Julian Seward [:jseward]]

Attachment: testcase.js as referred to in previous comment

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

I think there was some confusion about what I was originally proposing. Right now the way to conditionally trap with MIR is something like:

block0:
   compare
   test block1, block2

block1:
  trap

block2:
  ... continue

When this is a common pattern, this creates a ton of blocks and causes extra work for regalloc to handle liveness.

I'm suggesting that we create a new MIR node that encapsulates this 'side-exit' pattern: MWasmTrapIfZero [1] or MWasmTrapIfNull. This node would do the comparison and trapping if the input is null/zero, otherwise control flow will continue on. This would reduce the number of blocks we need.

This is less important once we have signal handling, but may still be useful if we can't eliminate all null-checks.

[1] https://searchfox.org/mozilla-central/rev/c33879cb4884c08c11980847cd84ccd76f485894/js/src/wasm/WasmIonCompile.cpp#4087-4113

Comment 5

[Julien Pages [:jpages]]

Attachment: Bug 1783956 - Add a new MIR node MWasmTrapIfNull. r=rhunt

This simplifies the control-flow when compiling ref.as_non_null
with the ion backend.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17d9f529b72a
Add a new MIR node MWasmTrapIfNull. r=rhunt

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/17d9f529b72a