Bug 173088

Summary: WebAssembly: We should only create wrappers for functions that can be exported
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Keith Miller
Reported 2017-06-07 21:43:14 PDT
WebAssembly: We should only create wrappers for functions that can be exported
Attachments
Patch (184.73 KB, patch)
2017-06-07 21:48 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch for landing (185.74 KB, patch)
2017-06-07 22:42 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch for landing (185.75 KB, patch)
2017-06-07 22:43 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch for landing (186.44 KB, patch)
2017-06-08 11:35 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2017-06-07 21:48:51 PDT
Created attachment 312277 [details] Patch
Saam Barati
Comment 2 2017-06-07 22:36:50 PDT
Comment on attachment 312277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312277&action=review r=me > Source/JavaScriptCore/ChangeLog:9 > + can actually be exported. It appears to be a ~2.5% speedup on WasmBench. nice > Source/JavaScriptCore/ChangeLog:11 > + This patch also removes most of the old testWasmModuleFunctions api from the jsc CLI. 🎉 Also, why "most" here? It looks like you're removing it all > Tools/Scripts/run-jsc-stress-tests:1204 > + if !$quickMode Do you want to give the specTests/Emscripten the same treatment?
Keith Miller
Comment 3 2017-06-07 22:41:00 PDT
Comment on attachment 312277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312277&action=review >> Source/JavaScriptCore/ChangeLog:11 >> + This patch also removes most of the old testWasmModuleFunctions api from the jsc CLI. > > 🎉 > > Also, why "most" here? It looks like you're removing it all The others were not removed but rather updated. >> Tools/Scripts/run-jsc-stress-tests:1204 >> + if !$quickMode > > Do you want to give the specTests/Emscripten the same treatment? Done!
Keith Miller
Comment 4 2017-06-07 22:42:21 PDT
Created attachment 312278 [details] Patch for landing
Keith Miller
Comment 5 2017-06-07 22:43:10 PDT
Created attachment 312279 [details] Patch for landing
Keith Miller
Comment 6 2017-06-07 22:43:32 PDT
Comment on attachment 312277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312277&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + can actually be exported. It appears to be a ~2.5% speedup on WasmBench. > > nice Whoops, I should have mentioned that it's only 2.5% on compile times.
Keith Miller
Comment 7 2017-06-08 11:35:01 PDT
Created attachment 312328 [details] Patch for landing
WebKit Commit Bot
Comment 8 2017-06-08 12:13:57 PDT
Comment on attachment 312328 [details] Patch for landing Clearing flags on attachment: 312328 Committed r217942: <http://trac.webkit.org/changeset/217942>
WebKit Commit Bot
Comment 9 2017-06-08 12:13:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.