Closed Bug 1495662 Opened 6 years ago Closed 6 years ago

--wasm-gc is not honored by JS shell with ENABLE_WASM_CRANELIFT

Categories

(Core :: JavaScript: WebAssembly, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file)

Superficially this seems to be a result of some new machinery introduced around a 'ScriptedCaller' object in the wasm subtree; this does not carry the required information and the shortfall is not made up by other logic.  I'm guessing this came in with Cranelift but I'm not sure yet.

This should have broken all wasm/gc test cases, except if our implementation of wasmGcSupported() is affected by the same problem.
Summary: --wasm-gc is no longer honored by JS shell → --wasm-gc is not honored by JS shell with ENABLE_WASM_CRANELIFT
Attachment #9013544 - Flags: review?(bbouvier)
Comment on attachment 9013544 [details] [diff] [review]
bug1495662-cranelift-and-gc-switches.patch

Review of attachment 9013544 [details] [diff] [review]:
-----------------------------------------------------------------

oops, my bad, sorry. Thanks for the fix!

::: js/src/shell/js.cpp
@@ +10064,5 @@
> +#if defined ENABLE_WASM_GC && defined ENABLE_WASM_CRANELIFT
> +    // Note, once we remove --wasm-gc this test will no longer make any sense
> +    // and we'll need a better solution.
> +    if (enableWasmGc && wasmForceCranelift) {
> +        fprintf(stderr, "Do not combine --wasm-gc and --wasm-force-cranelift, they are incompatible.\n");

Alternatively, we could just have `enableWasmGc = op.getBoolOption("wasm-gc") && !wasmForceCranelift;`. Your solution works fine too.
Attachment #9013544 - Flags: review?(bbouvier) → review+
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16385c9bc942
cranelift presence must not disable wasm-gc.  r=bbouvier
https://hg.mozilla.org/mozilla-central/rev/16385c9bc942
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: