Bug 169998

Summary: [JSC][DFG] Make addShouldSpeculateAnyInt more conservative to avoid regression caused by Double <-> Int52 conversions
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, fpizlo, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch
none
Patch
none
Patch none

Yusuke Suzuki
Reported 2017-03-23 00:42:42 PDT
To fix imaging-darkroom regression (a bit problem is that it is not shown in my Linux box right now)...
Attachments
Patch (3.54 KB, patch)
2017-03-23 01:12 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (16.53 MB, application/zip)
2017-03-23 02:51 PDT, Build Bot 		no flags
Details
Patch (3.96 KB, patch)
2017-03-23 05:22 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (5.47 KB, patch)
2017-03-23 12:26 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (9.10 KB, patch)
2017-03-23 13:20 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2017-03-23 01:12:27 PDT
Created attachment 305176 [details] Patch
Build Bot
Comment 2 2017-03-23 02:51:07 PDT
Comment on attachment 305176 [details] Patch Attachment 305176 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3393619 New failing tests: perf/object-keys.html
Build Bot
Comment 3 2017-03-23 02:51:10 PDT
Created attachment 305179 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Yusuke Suzuki
Comment 4 2017-03-23 05:22:05 PDT
Created attachment 305184 [details] Patch
Yusuke Suzuki
Comment 5 2017-03-23 11:59:06 PDT
Comment on attachment 305184 [details] Patch I'll make this speculation more a bit conservative since Double to Int52 is not so cheap.
Yusuke Suzuki
Comment 6 2017-03-23 12:26:31 PDT
Created attachment 305213 [details] Patch
Yusuke Suzuki
Comment 7 2017-03-23 12:27:11 PDT
OK, ready.
Yusuke Suzuki
Comment 8 2017-03-23 12:52:34 PDT
Comment on attachment 305213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305213&action=review > Source/JavaScriptCore/dfg/DFGGraph.h:313 > + return (left->shouldSpeculateAnyInt() || right->shouldSpeculateAnyInt()) I think we should see NodeFlags here.
Yusuke Suzuki
Comment 9 2017-03-23 13:20:49 PDT
Created attachment 305220 [details] Patch
Yusuke Suzuki
Comment 10 2017-03-23 13:22:34 PDT
Comment on attachment 305220 [details] Patch OK! Now, our addShouldSpeculateAnyInt has more solid and reasonable story.
Saam Barati
Comment 11 2017-03-23 14:31:01 PDT
Comment on attachment 305220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305220&action=review r=me > Source/JavaScriptCore/ChangeLog:60 > + baseline patched > + > + imaging-darkroom 201.112+-3.192 ^ 189.532+-2.883 ^ definitely 1.0611x faster > + stanford-crypto-pbkdf2 103.953+-2.325 100.926+-2.396 might be 1.0300x faster > + stanford-crypto-sha256-iterative 35.103+-1.071 ? 36.049+-1.143 ? might be 1.0270x slower > + Nice
Yusuke Suzuki
Comment 12 2017-03-23 15:24:04 PDT
Comment on attachment 305220 [details] Patch Thanks!
WebKit Commit Bot
Comment 13 2017-03-23 15:53:57 PDT
Comment on attachment 305220 [details] Patch Clearing flags on attachment: 305220 Committed r214323: <http://trac.webkit.org/changeset/214323>
WebKit Commit Bot
Comment 14 2017-03-23 15:54:02 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.