Closed Bug 1488162 Opened 2 years ago Closed 2 years ago

Remove suppressGC hacks from wasm code

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: lth, Assigned: jseward)

References

Details

Attachments

(1 file, 2 obsolete files)

Once we have stack maps and tracing we no longer need to suppress GC when wasm activations are on the stack.
Blocks: 1488205
No longer blocks: 1487329
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().
Assignee: nobody → jseward
Status: NEW → ASSIGNED
Attached patch bug1488162-rm-suppressGC-1.diff (obsolete) — Splinter Review
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.
Attachment #9018330 - Flags: review?(lhansen)
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?
Attachment #9018330 - Flags: review?(lhansen) → review+
Attached patch bug1488162-rm-suppressGC-7.diff (obsolete) — Splinter Review
Updated patch.
Attachment #9018330 - Attachment is obsolete: true
Rebased to 449244:0ae2634de8e1.
Attachment #9026419 - Attachment is obsolete: true
Pushed by jseward@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9e2ee18925b
Remove suppressGC hacks from wasm code.  r=lth.
https://hg.mozilla.org/mozilla-central/rev/d9e2ee18925b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.