1508561 - Update ifdeffery so that we can ship reftypes without shipping GC, struct types, (ref T) types, et al

Status: RESOLVED FIXED

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

Description

[Lars T Hansen [:lth]]

At the moment, we've lumped a lot of things in together for the sake of experimentation, but if we want to ship reftypes without shipping struct types and (ref T) types and misc other in-flight cruft we have to slice things a little differently, so there's a job to be done here to reorganize things properly.

Comment 1

[Lars T Hansen [:lth]]

I should add, this is complicated a little extra by perhaps wanting to have reftypes on by default on Nightly but not on beta / release (until the proposal is closer to being final), and / or wishing to have a run-time switch.

Comment 2

[Lars T Hansen [:lth]]

We'll need to slice the test cases too.  Bug 1508559 is starting to feel the pain of having GC test cases lumped in with anyref test cases.

Comment 3

[Lars T Hansen [:lth]]

This is blocking me now so I'm just going to do it; it's a good time, since stack maps have landed.

Comment 4

[Lars T Hansen [:lth]]

Attachment: bug1508561-separate-test-cases.patch

Test case separation WIP.  This could in principle land, but can't because that would reduce test coverage somewhat, since we now want to run with both --no-wasm-ion and --no-wasm-baseline, and we can't quite do that:

We're waiting for either bug 1488205 to remove --wasm-gc so that we can put --no-wasm-ion and --no-wasm-baseline in its place in directives.txt, or bug 1515085 so that we can have multiple long args in the test-also directive.

Comment 5

[Lars T Hansen [:lth]]

Actually we're waiting for bug 1515917 to fix some bugs that were found as a result of combining flags.

Comment 6

[Lars T Hansen [:lth]]

Attachment: bug1508561-disentangle-reftypes-and-gctypes.patch

Complete disentanglement of test cases and code, though with some exceptions noted in the commit msg, I hope for good reason.

This sits on top of my patch for bug 1515917, which will land shortly.

After this lands, we'll all have to learn to distinguish between ENABLE_WASM_GC and ENABLE_WASM_REFTYPES.

Comment 7

[Julian Seward [:jseward]]

Comment on attachment 9033968 [details] [diff] [review]
bug1508561-disentangle-reftypes-and-gctypes.patch

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

This looks OK to me.

> - Internal names still use "Gc" rather than "Reftypes", eg,
>   HasGcTypes, wasmGc_, and so on.  This is most appropriate since it
>   reduces the scope of the patch and these names will pertain mainly
>   to the gc feature in the future.

This was indeed the one confusing feature of the patch.  For those names
that won't pertain to the gc in future, is it worth filing a followup so
as to rename them accordingly?

Comment 8

[Lars T Hansen [:lth]]

Huh. Some of the ifdeffery is used elsewhere in the browser, and other modules have their own definitions of ENABLE_WASM_GC and so need definitions of ENABLE_WASM_REFTYPES too. Fairly sure that's not great, a single config would have been better.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/baf99d3bdfea
Disentangle support for reftypes and gc.  r=jseward

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/baf99d3bdfea