1805856 - Avoid reading wasm options from JSContext while parsing asm.js

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

Currently wasm-related options (CompileArgs, FeatureArgs) are accessed at the beginning of asm.js compilation, which happens in the middle of compilation.

https://searchfox.org/mozilla-central/rev/43cb6eca1c3069d46d589d52ab4949257e630d19/js/src/wasm/WasmCompile.cpp#84-150

To make compilation JSContext-free, those information should be copied to CompileOptions before starting the compilation, and asm.js compilation should get those information from CompileOptions.

Comment 1

[Ted Campbell [:tcampbell]]

I'll take this and try to both clean up the WASM options handling and add the stuff that I cannot remove to CompileOptions.

Comment 2

[Ted Campbell [:tcampbell]]

For AsmJS, we don't support any features or interesting compile options, so I'll be able to add a CompileArgs::buildForAsmJS entry point that reduces dependency on JSContext.

The one ugly part that I'm still trying to untangle is debugging. This should be decided up front in the compile options and not read again later from the cx, but I'll need to do some tweaks to WASM code.

Unfortunately, the pre-existing code has numerous defects around this stuff which I'm trying to clean up as well.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1805856 - Use default WASM compile options for legacy asm.js. r?rhunt

In order to decouple the JS Parser from the JSContext, we want to avoid reading
the WASM feature flags during asm.js compile. Fortunately, asm.js doesn't use
new WASM features or the WASM debugger mechanism, so we can add a new constructor
for wasm::CompileArgs to computes the fixed options for asm.js without needing
a JSContext argument.

Comment 4

[Ted Campbell [:tcampbell]]

I was able to address the immediate concern (reading feature flags within asm.js parsing) without needing to fix the flags + debugger issues in this bug. In Bug 1806499 I was looking at removing JSContext from wasm::HasPlatfromSupport which might also be needed still to remove more uses of JSContext.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b0f4e0e75ca
Use default WASM compile options for legacy asm.js. r=rhunt

Comment 6

[Cristian Tuns]

Backed out for causing spidermonkey bustages

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/js/src/wasm/AsmJS.cpp:2189:25: error: use of undeclared identifier 'ec_'; did you mean 'fc_'?
  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:668: Unified_cpp_js_src_wasm0.o] Error 1

Comment 7

[Ted Campbell [:tcampbell]]

Oops. Merge conflict with Bug 1787528. I'll update and reland.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60af383da56b
Use default WASM compile options for legacy asm.js. r=rhunt

Comment 9

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/60af383da56b