1488162 - Remove suppressGC hacks from wasm code

Status: RESOLVED FIXED

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

Description

[Lars T Hansen [:lth]]

Once we have stack maps and tracing we no longer need to suppress GC when wasm activations are on the stack.

Comment 1

[Lars T Hansen [:lth]]

In addition to obvious uses in WasmStubs.cpp, also note weird cases like GCRuntime::temporaryAbortIfWasmGc(), we should be able to track them all down by looking for uses of JS::ContextOptions::wasmGc().

Comment 2

[Julian Seward [:jseward]]

Attachment: bug1488162-rm-suppressGC-1.diff

This removes GCRuntime::temporaryAbortIfWasmGc() and SuppressGC and
all the C++ fragments guarded by them, and also the 7 associated test files.
I also checked all uses of JS::ContextOptions::wasmGc() but didn't find
any further C++ frags guarded by it.

Comment 3

[Lars T Hansen [:lth]]

Comment on attachment 9018330 [details] [diff] [review]
bug1488162-rm-suppressGC-1.diff

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

This seems fine.  I assume the --wasm-gc flag and all the variables propagating that through the engine will be removed on the other bug.

::: js/src/wasm/WasmStubs.cpp
@@ -356,5 @@
>          masm.loadPtr(Address(masm.getStackPointer(), argBase + arg.offsetFromArgBase()), WasmTlsReg);
>      }
>  
>  #ifdef ENABLE_WASM_GC
>      WasmPush(masm, WasmTlsReg);

Any reason you can't remove this line, the corresponding pop, and then both of the ifdef blocks altogether?

Comment 4

[Julian Seward [:jseward]]

Attachment: bug1488162-rm-suppressGC-7.diff

Updated patch.

Comment 5

[Julian Seward [:jseward]]

Attachment: bug1488162-rm-suppressGC-10.diff

Rebased to 449244:0ae2634de8e1.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9e2ee18925b
Remove suppressGC hacks from wasm code.  r=lth.

Comment 7

[Cosmin Sabou [:CosminS]]

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