Chromium Code Reviews

Issue 2313073002: [builtins] Migrate Number predicates and make them optimizable. (Closed)

Created:
4 years, 3 months ago by Benedikt Meurer
Modified:
4 years, 3 months ago
Reviewers:
Franzi, Benedikt Meurer (Google)
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[builtins] Migrate Number predicates and make them optimizable. Migrate the isNaN, isFinite, Number.isFinite, Number.isInteger, Number.isSafeInteger and Number.isNaN predicates to TurboFan builtins and make them optimizable (for certain input types) in JavaScript callees being optimized by TurboFan. That means both the baseline and the optimized version is now always at maximum, consistent performance. Especially TurboFan suffered from poor baseline (and optimized) performance because it cannot play the same weird tricks that Crankshaft plays for %_IsSmi. This also adds a bunch of new tests to properly cover the use of the Harmony predicates in optimized code. R=franzih@chromium.org BUG=v8:5049, v8:5267 Committed: https://crrev.com/7ac19fe59823c82a8cf484d50a26767e1c828572 Cr-Commit-Position: refs/heads/master@{#39242}

Patch Set 1 #

Unified diffs Side-by-side diffs Stats (+733 lines, -80 lines)
M src/bootstrapper.cc View 2 chunks +17 lines, -0 lines 0 comments
M src/builtins/builtins.h View 2 chunks +13 lines, -3 lines 0 comments
M src/builtins/builtins-global.cc View 2 chunks +109 lines, -0 lines 0 comments
M src/builtins/builtins-number.cc View 1 chunk +138 lines, -0 lines 0 comments
M src/compiler/js-builtin-reducer.h View 2 chunks +6 lines, -0 lines 0 comments
M src/compiler/js-builtin-reducer.cc View 4 chunks +100 lines, -0 lines 0 comments
M src/compiler/typer.cc View 2 chunks +8 lines, -0 lines 0 comments
M src/js/collection.js View 4 chunks +4 lines, -6 lines 0 comments
M src/js/i18n.js View 3 chunks +2 lines, -3 lines 0 comments
M src/js/prologue.js View 1 chunk +0 lines, -2 lines 0 comments
M src/js/typedarray.js View 3 chunks +3 lines, -5 lines 0 comments
M src/js/v8natives.js View 5 chunks +0 lines, -56 lines 0 comments
M src/objects.h View 2 chunks +6 lines, -0 lines 0 comments
M test/cctest/test-serialize.cc View 3 chunks +5 lines, -5 lines 0 comments
A test/mjsunit/compiler/number-isfinite.js View 1 chunk +29 lines, -0 lines 0 comments
A test/mjsunit/compiler/number-isinteger.js View 1 chunk +30 lines, -0 lines 0 comments
A test/mjsunit/compiler/number-isnan.js View 1 chunk +28 lines, -0 lines 0 comments
A test/mjsunit/compiler/number-issafeinteger.js View 1 chunk +50 lines, -0 lines 0 comments
M test/unittests/compiler/js-builtin-reducer-unittest.cc View 3 chunks +185 lines, -0 lines 0 comments

Messages

Total messages: 12 (6 generated)
Benedikt Meurer
4 years, 3 months ago (2016-09-06 20:42:04 UTC) #1
Benedikt Meurer
Hey Franziska, Here's some refactoring for 6 number predicates, 2 on the global object and ...
4 years, 3 months ago (2016-09-06 20:45:09 UTC) #4
Franzi
lgtm
4 years, 3 months ago (2016-09-07 08:40:06 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2313073002/1
4 years, 3 months ago (2016-09-07 10:11:58 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 3 months ago (2016-09-07 10:14:23 UTC) #10
commit-bot: I haz the power
4 years, 3 months ago (2016-09-07 10:14:53 UTC) #12
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/7ac19fe59823c82a8cf484d50a26767e1c828572
Cr-Commit-Position: refs/heads/master@{#39242}

Powered by Google App Engine