Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(29)

Issue 1841513003: [crankshaft] Address the deoptimization loops of Math.floor, Math.round and Math.ceil. (Closed)

Created:
4 years, 8 months ago by Benedikt Meurer
Modified:
4 years, 8 months ago
Reviewers:
Jarin
CC:
v8-reviews_googlegroups.com, v8-x87-ports_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[crankshaft] Address the deoptimization loops of Math.floor, Math.round and Math.ceil. Fix and re-enable the flexible representation for Math.floor (which is used to implement Math.ceil) and Math.round, which allows Math.floor and Math.round to return double results instead of int32, and therefore allows values outside the int32 range, especially -0 is now a valid result, which doesn't deopt. Also port this feature to x64 and ia32 when the CPU supports the SSE4.1 extension. This addresses all the known deoptimization loops related to Math.round in the Kraken benchmark suite, and seems to also address most of the deoptimization loops related to Math.floor in the Oort Online benchmark. Drive-by-fix: Import the regression tests for the broken HMathFloorOfDiv optimization that caused the initial revert of the feature (for arm64 only back then). BUG=chromium:476477, v8:2890, v8:4059 R=jarin@chromium.org LOG=n Committed: https://crrev.com/978ad03b9275fb338f6dd484c1ef732d1a97ccfe Cr-Commit-Position: refs/heads/master@{#35094}

Patch Set 1 #

Patch Set 2 : Register constraints in Crankshaft are fun #

Unified diffs Side-by-side diffs Delta from patch set Stats (+182 lines, -55 lines) Patch
M src/crankshaft/hydrogen-instructions.h View 1 chunk +4 lines, -4 lines 0 comments Download
M src/crankshaft/hydrogen-instructions.cc View 1 chunk +2 lines, -1 line 0 comments Download
M src/crankshaft/ia32/lithium-codegen-ia32.cc View 2 chunks +23 lines, -2 lines 0 comments Download
M src/crankshaft/ia32/lithium-ia32.h View 4 chunks +32 lines, -11 lines 0 comments Download
M src/crankshaft/ia32/lithium-ia32.cc View 1 1 chunk +19 lines, -8 lines 0 comments Download
M src/crankshaft/x64/lithium-codegen-x64.cc View 2 chunks +23 lines, -2 lines 0 comments Download
M src/crankshaft/x64/lithium-x64.h View 4 chunks +32 lines, -11 lines 0 comments Download
M src/crankshaft/x64/lithium-x64.cc View 1 1 chunk +19 lines, -8 lines 0 comments Download
A test/mjsunit/regress/regress-crbug-476477-1.js View 1 chunk +21 lines, -0 lines 0 comments Download
A + test/mjsunit/regress/regress-crbug-476477-2.js View 1 chunk +7 lines, -8 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
Benedikt Meurer
4 years, 8 months ago (2016-03-29 09:29:35 UTC) #1
Benedikt Meurer
Hey Jaro, Here's the baby we talked about; it's actually quite simple (since the ARM ...
4 years, 8 months ago (2016-03-29 09:31:03 UTC) #2
Jarin
lgtm
4 years, 8 months ago (2016-03-29 10:21:49 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1841513003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1841513003/20001
4 years, 8 months ago (2016-03-29 10:22:24 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2016-03-29 10:24:36 UTC) #6
commit-bot: I haz the power
4 years, 8 months ago (2016-03-29 10:25:09 UTC) #8
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/978ad03b9275fb338f6dd484c1ef732d1a97ccfe
Cr-Commit-Position: refs/heads/master@{#35094}

Powered by Google App Engine
This is Rietveld 408576698