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: | WebAssembly | Assignee: | 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 |
Tim Chevalier
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.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Tim Chevalier
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.
David Degazio
rdar://106039216
David Degazio
Pull request: https://github.com/WebKit/WebKit/pull/10809
EWS
Committed 260960@main (e172f58e7b57): <https://commits.webkit.org/260960@main>
Reviewed commits have been landed. Closing PR #10809 and removing active labels.