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

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: lth, Assigned: lth)

Tracking

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

9 months ago
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.
Assignee

Updated

9 months ago
Summary: --wasm-gc is no longer honored by JS shell → --wasm-gc is not honored by JS shell with ENABLE_WASM_CRANELIFT
Assignee

Comment 1

9 months ago
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+

Comment 3

9 months ago
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16385c9bc942
cranelift presence must not disable wasm-gc.  r=bbouvier

Comment 4

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/16385c9bc942
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.