...
Created attachment 384095 [details] Patch
Created attachment 384229 [details] Patch
Created attachment 384236 [details] Patch Fix CMake build
Comment on attachment 384236 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384236&action=review r=me > Source/JavaScriptCore/ChangeLog:13 > + This is a 1.5x speedup when compiling the ZenGarden demo. what do we do for the .validate API? (You should say here.) > Source/JavaScriptCore/ChangeLog:14 > + you should talk about the new policy that WebAssembly.module compiles bytecode > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:237 > + static_assert(std::is_same_v<ResultList, FunctionParser<AirIRGenerator>::ResultList>); why not just define one to be the other? > Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:89 > + } maybe ASSERT here that m_state == Validated > Source/JavaScriptCore/wasm/WasmFunctionParser.h:146 > +#define WASM_VALIDATOR_FAIL_IF(condition, ...) do { \ > + if (UNLIKELY(condition)) \ > + return validationFail(__VA_ARGS__); \ > + } while (0) style nit: line up the "\" or have them be one space after > Source/JavaScriptCore/wasm/WasmFunctionParser.h:288 > + WASM_VALIDATOR_FAIL_IF(pointer.type() != I32, m_currentOpcode, " pointer type mismatch"); maybe instead of "type mismatch" say " pointer type must be an i32"? (This is probably not new to your patch though, so feel free to ignore) > Source/JavaScriptCore/wasm/WasmFunctionParser.h:311 > + WASM_VALIDATOR_FAIL_IF(pointer.type() != I32, m_currentOpcode, " pointer type mismatch"); ditto > Source/JavaScriptCore/wasm/WasmLLIntPlan.cpp:58 > +{ ASSERT m_callees?
Comment on attachment 384236 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384236&action=review Thanks for the review! >> Source/JavaScriptCore/ChangeLog:13 >> + This is a 1.5x speedup when compiling the ZenGarden demo. > > what do we do for the .validate API? (You should say here.) done >> Source/JavaScriptCore/ChangeLog:14 >> + > > you should talk about the new policy that WebAssembly.module compiles bytecode done >> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:237 >> + static_assert(std::is_same_v<ResultList, FunctionParser<AirIRGenerator>::ResultList>); > > why not just define one to be the other? I want it to be defined in FunctionParser, and we can't define it here as FunctionParser<AirIRGenerator>::ResultList because that will fail to instantiate FunctionParser, since that requires ControlType to be defined and ControlType depends on ResultList. >> Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:89 >> + } > > maybe ASSERT here that m_state == Validated added >> Source/JavaScriptCore/wasm/WasmFunctionParser.h:146 >> + } while (0) > > style nit: line up the "\" or have them be one space after fixed >> Source/JavaScriptCore/wasm/WasmFunctionParser.h:288 >> + WASM_VALIDATOR_FAIL_IF(pointer.type() != I32, m_currentOpcode, " pointer type mismatch"); > > maybe instead of "type mismatch" say " pointer type must be an i32"? > (This is probably not new to your patch though, so feel free to ignore) I will fix this in a separate patch since there are a lot of existing code that needs fixing. >> Source/JavaScriptCore/wasm/WasmLLIntPlan.cpp:58 >> +{ > > ASSERT m_callees? added
Created attachment 384859 [details] Patch for landing
Comment on attachment 384859 [details] Patch for landing Rejecting attachment 384859 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 384859, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 5000 characters of output: eeded at 1 with fuzz 3. patching file Source/JavaScriptCore/CMakeLists.txt Hunk #1 succeeded at 366 (offset -2 lines). patching file Source/JavaScriptCore/DerivedSources-input.xcfilelist patching file Source/JavaScriptCore/DerivedSources-output.xcfilelist Hunk #1 succeeded at 60 with fuzz 1 (offset -1 lines). patching file Source/JavaScriptCore/DerivedSources.make Hunk #2 succeeded at 353 (offset 2 lines). patching file Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp Hunk #18 succeeded at 1799 (offset 77 lines). Hunk #19 succeeded at 1830 (offset 77 lines). Hunk #20 succeeded at 1843 (offset 77 lines). Hunk #21 succeeded at 1861 (offset 77 lines). Hunk #22 succeeded at 1877 (offset 77 lines). Hunk #23 succeeded at 1898 (offset 77 lines). Hunk #24 succeeded at 1936 (offset 77 lines). Hunk #25 succeeded at 2051 (offset 77 lines). Hunk #26 succeeded at 2068 (offset 77 lines). Hunk #27 succeeded at 2093 (offset 77 lines). Hunk #28 succeeded at 2183 (offset 77 lines). Hunk #29 succeeded at 2339 (offset 77 lines). Hunk #30 succeeded at 2357 (offset 77 lines). Hunk #31 succeeded at 2382 (offset 77 lines). Hunk #32 succeeded at 2405 (offset 77 lines). Hunk #33 succeeded at 2994 (offset 77 lines). Hunk #34 succeeded at 3069 (offset 77 lines). patching file Source/JavaScriptCore/wasm/WasmAirIRGenerator.h patching file Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp Hunk #14 succeeded at 1291 (offset 68 lines). Hunk #15 succeeded at 1354 (offset 68 lines). Hunk #16 succeeded at 1390 (offset 68 lines). Hunk #17 succeeded at 1409 (offset 68 lines). Hunk #18 succeeded at 1437 (offset 68 lines). Hunk #19 succeeded at 1456 (offset 68 lines). Hunk #20 succeeded at 1476 (offset 68 lines). Hunk #21 succeeded at 1547 (offset 68 lines). Hunk #22 succeeded at 1568 (offset 68 lines). Hunk #23 succeeded at 1919 (offset 68 lines). Hunk #24 succeeded at 1954 (offset 68 lines). Hunk #25 succeeded at 1978 (offset 68 lines). patching file Source/JavaScriptCore/wasm/WasmB3IRGenerator.h patching file Source/JavaScriptCore/wasm/WasmBBQPlan.cpp patching file Source/JavaScriptCore/wasm/WasmBBQPlan.h patching file Source/JavaScriptCore/wasm/WasmCodeBlock.cpp patching file Source/JavaScriptCore/wasm/WasmCodeBlock.h patching file Source/JavaScriptCore/wasm/WasmEntryPlan.cpp patching file Source/JavaScriptCore/wasm/WasmEntryPlan.h patching file Source/JavaScriptCore/wasm/WasmFunctionParser.h patching file Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp Hunk #1 succeeded at 49 (offset 1 line). Hunk #2 succeeded at 72 (offset 1 line). Hunk #3 succeeded at 92 (offset 1 line). Hunk #4 succeeded at 105 (offset 1 line). Hunk #5 succeeded at 137 (offset 1 line). Hunk #6 succeeded at 154 (offset 1 line). Hunk #7 succeeded at 182 (offset 1 line). Hunk #8 succeeded at 234 (offset 1 line). Hunk #9 succeeded at 250 (offset 1 line). Hunk #10 succeeded at 293 (offset 1 line). Hunk #11 succeeded at 363 (offset 1 line). Hunk #12 succeeded at 408 (offset 1 line). Hunk #13 succeeded at 433 (offset 1 line). Hunk #14 succeeded at 525 (offset 1 line). Hunk #15 succeeded at 590 (offset 1 line). Hunk #16 succeeded at 696 (offset 1 line). Hunk #17 succeeded at 756 (offset 1 line). Hunk #18 succeeded at 806 (offset 20 lines). Hunk #19 succeeded at 825 (offset 20 lines). Hunk #20 succeeded at 843 (offset 20 lines). Hunk #21 succeeded at 853 (offset 20 lines). Hunk #22 succeeded at 973 (offset 20 lines). Hunk #23 succeeded at 1005 (offset 20 lines). Hunk #24 succeeded at 1019 (offset 20 lines). patching file Source/JavaScriptCore/wasm/WasmLLIntGenerator.h patching file Source/JavaScriptCore/wasm/WasmLLIntPlan.cpp patching file Source/JavaScriptCore/wasm/WasmLLIntPlan.h patching file Source/JavaScriptCore/wasm/WasmModule.cpp patching file Source/JavaScriptCore/wasm/WasmModule.h patching file Source/JavaScriptCore/wasm/WasmOMGPlan.cpp patching file Source/JavaScriptCore/wasm/WasmPlan.cpp patching file Source/JavaScriptCore/wasm/WasmPlan.h patching file Source/JavaScriptCore/wasm/WasmSlowPaths.cpp Hunk #1 succeeded at 411 (offset 7 lines). patching file Source/JavaScriptCore/wasm/WasmThunks.cpp patching file Source/JavaScriptCore/wasm/WasmThunks.h patching file Source/JavaScriptCore/wasm/WasmValidate.cpp Hunk #4 FAILED at 170. 1 out of 8 hunks FAILED -- saving rejects to file Source/JavaScriptCore/wasm/WasmValidate.cpp.rej patching file Source/JavaScriptCore/wasm/WasmWorklist.cpp patching file Source/JavaScriptCore/wasm/generateWasmOpsHeader.py patching file Source/JavaScriptCore/wasm/generateWasmValidateInlinesHeader.py rm 'Source/JavaScriptCore/wasm/generateWasmValidateInlinesHeader.py' patching file Source/JavaScriptCore/wasm/js/JSWebAssembly.cpp Hunk #1 succeeded at 229 (offset 1 line). Hunk #2 succeeded at 329 (offset 1 line). patching file Source/JavaScriptCore/wasm/js/WebAssemblyInstanceConstructor.cpp Hunk #1 succeeded at 81 with fuzz 1. Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/13286559
Created attachment 384874 [details] Patch for landing
Comment on attachment 384874 [details] Patch for landing Clearing flags on attachment: 384874 Committed r253140: <https://trac.webkit.org/changeset/253140>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57652345>
It looks like the changes in https://trac.webkit.org/changeset/253140/webkit caused 11 tests to crash on Mac debug wk1. tracking in https://bugs.webkit.org/show_bug.cgi?id=204893