Bug 252722

Summary: [Wasm-GC] wasm-b3 test config in run-jsc-stress-tests script doesn't run B3
Product: WebKit Reporter: Tim Chevalier <tjc>
Component: WebAssemblyAssignee: David Degazio <d_degazio>
Status: RESOLVED FIXED    
Severity: Normal CC: d_degazio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 247394    

Description Tim Chevalier 2023-02-21 21:30:38 PST 
I noticed while isolating bug 252719 that the run-jsc-stress-tests script doesn't cause B3 to be invoked when running the "wasm-b3" config. "wasm-b3" amounts to the following command-line options:

```
--useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useWasmLLInt\=false --wasmBBQUsesAir\=false --useFTLJIT\=true
```

The `BBQPlan::planGeneratesLoopOSREntrypoints()` method ( https://searchfox.org/wubkat/source/Source/JavaScriptCore/wasm/WasmBBQPlan.cpp#64 ) returns true unless either the `--webAssemblyBBQAirModeThreshold` option or the `--useSinglePassBBQJIT` option is present. `BBQPlan::compileFunction()` ( https://searchfox.org/wubkat/source/Source/JavaScriptCore/wasm/WasmBBQPlan.cpp#276 ) only invokes B3 if that method returns false. So when running tests with the wasm-b3 config, B3 is never invoked, which seems like it's unintentional.

I verified this by editing WasmB3IRGenerator to replace all the generated code for array and struct operations with `addCrash()` calls and running `run-jsc-stress-tests --filter wasm/gc`. All the tests still passed, suggesting that B3 code isn't being compiled/run with any of the combinations of options that run-jsc-stress-tests uses.

I was able to force B3 to be invoked by adding `--webAssemblyBBQAirModeThreshold=1` to the options for wasm-b3 in the run-jsc-stress-tests script, but I don't know if that's the right solution.
Comment 1 Tim Chevalier 2023-02-23 15:45:44 PST 
Asumu suggested that the change might be due to this commit from last week:

https://github.com/WebKit/WebKit/commit/46375fbc5a6710ec3e2fd10848009e31b307b741

In particular, this change to `BBQPlan::planGeneratesLoopOSREntrypoints()`:

https://github.com/WebKit/WebKit/commit/46375fbc5a6710ec3e2fd10848009e31b307b741#diff-2f666bcf8afdb33c316380e643aa65ede3fa2d8ee0dfa57a3d45fca00fcd4d9dL72

changes the meaning of the `--wasmBBQUsesAir` flag, which the run-jsc-stress-test script uses to enable B3.

If I add this to the top of the method:

```
    if (!Options::wasmBBQUsesAir())
        return false;
```

then B3 does run in the wasm-b3 config. But I'm not sure if that's the right solution.
Comment 2 David Degazio 2023-02-28 11:19:43 PST 
rdar://106039216
Comment 3 David Degazio 2023-02-28 11:27:30 PST 
Pull request: https://github.com/WebKit/WebKit/pull/10809
Comment 4 EWS 2023-02-28 12:30:57 PST 
Committed 260960@main (e172f58e7b57): <https://commits.webkit.org/260960@main>

Reviewed commits have been landed. Closing PR #10809 and removing active labels.