Bug 169998 - [JSC][DFG] Make addShouldSpeculateAnyInt more conservative to avoid regression caused by Double <-> Int52 conversions
Summary: [JSC][DFG] Make addShouldSpeculateAnyInt more conservative to avoid regressio...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-23 00:42 PDT by Yusuke Suzuki
Modified: 2017-03-23 15:54 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.54 KB, patch)
2017-03-23 01:12 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
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 Details | Formatted Diff | Diff
Patch (5.47 KB, patch)
2017-03-23 12:26 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (9.10 KB, patch)
2017-03-23 13:20 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 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)...
Comment 1 Yusuke Suzuki 2017-03-23 01:12:27 PDT
Created attachment 305176 [details]
Patch
Comment 2 Build Bot 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
Comment 3 Build Bot 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
Comment 4 Yusuke Suzuki 2017-03-23 05:22:05 PDT
Created attachment 305184 [details]
Patch
Comment 5 Yusuke Suzuki 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.
Comment 6 Yusuke Suzuki 2017-03-23 12:26:31 PDT
Created attachment 305213 [details]
Patch
Comment 7 Yusuke Suzuki 2017-03-23 12:27:11 PDT
OK, ready.
Comment 8 Yusuke Suzuki 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.
Comment 9 Yusuke Suzuki 2017-03-23 13:20:49 PDT
Created attachment 305220 [details]
Patch
Comment 10 Yusuke Suzuki 2017-03-23 13:22:34 PDT
Comment on attachment 305220 [details]
Patch

OK! Now, our addShouldSpeculateAnyInt has more solid and reasonable story.
Comment 11 Saam Barati 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
Comment 12 Yusuke Suzuki 2017-03-23 15:24:04 PDT
Comment on attachment 305220 [details]
Patch

Thanks!
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2017-03-23 15:54:02 PDT
All reviewed patches have been landed.  Closing bug.