Bug 277703

Summary: [WASM] Subsequent comparisons and conditional branches should be fused in BBQ
Product: WebKit Reporter: David Degazio <d_degazio>
Component: WebAssemblyAssignee: David Degazio <d_degazio>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   

David Degazio
Reported 2024-08-06 12:39:19 PDT
Since WASM only has br_if and if - which branch on a boolean i32 value - as conditional branch primitives, it's common in WASM to write sequences like these: local.get 0 i32.eqz br_if 0 local.get 1 i32.const 10 i32.lt_s if In BBQ, since we compile in a single pass, we currently handle these cases super naively, emitting each opcode independently. So for that first example with the eqz, we'll emit something like the following on ARM: ldur w0, [fp, #-12] cmp w0, #0 cset w0, eq mov x8, x0 cbz w8, ... This is pretty nasty - we're using cset to store the result of comparing our local against 0, only to use it in a cbz a few instructions later. In a single-pass compiler our options for improving on this are a little bit limited. We could consider some kind of abstract representation of a condition, making partially-evaluated conditions a member of BBQJIT::Value. This has a lot of weird implications for the register allocator and I'm not sure it's super worth it. In the absence of that, we should at least combine obviously combinable instructions like these that are literally adjacent in the source. Due to WASM's stack-based nature, this is almost always how such comparisons are emitted, so even a simple approach like one bytecode of lookahead should capture many of the combinable cases.
Attachments
Radar WebKit Bug Importer
Comment 1 2024-08-06 12:39:29 PDT
David Degazio
Comment 2 2024-08-06 13:03:26 PDT
EWS
Comment 3 2024-08-27 10:16:49 PDT
Committed 282799@main (406449de0ae5): <https://commits.webkit.org/282799@main> Reviewed commits have been landed. Closing PR #31802 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.