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

Issue 2299883003: [turbofan] Float32Constant/Float64Constant cannot occur in JS level graph. (Closed)

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

Description

[turbofan] Float32Constant/Float64Constant cannot occur in JS level graph. Now that the hole NaN is no longer represented as Float64Constant early on, we should never see such a constant node in any JS-level graph, but we will only see them after representation selection. Change Typer and SimplifiedLowering appropriately (and fix the invalid tests). R=jarin@chromium.org BUG=v8:5267 Committed: https://crrev.com/83e14103203f581b9b741d2b0612374e88855625 Cr-Commit-Position: refs/heads/master@{#39063}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -91 lines) Patch
M src/compiler/simplified-lowering.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M src/compiler/typer.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M test/cctest/compiler/test-js-constant-cache.cc View 10 chunks +0 lines, -79 lines 0 comments Download
M test/cctest/compiler/test-js-typed-lowering.cc View 1 chunk +3 lines, -4 lines 0 comments Download
M test/unittests/compiler/effect-control-linearizer-unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 15 (8 generated)
Benedikt Meurer
4 years, 3 months ago (2016-09-01 05:19:14 UTC) #1
Benedikt Meurer
Hey Jaro, Now we can cleanup the Float32/Float64Constant mess further and do proper checking. Please ...
4 years, 3 months ago (2016-09-01 05:20:05 UTC) #4
Jarin
lgtm
4 years, 3 months ago (2016-09-01 05:44:48 UTC) #7
commit-bot: I haz the power
This CL has an open dependency (Issue 2303643002 Patch 1). Please resolve the dependency and ...
4 years, 3 months ago (2016-09-01 05:59:57 UTC) #10
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/2299883003/1
4 years, 3 months ago (2016-09-01 06:24:26 UTC) #12
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 3 months ago (2016-09-01 06:26:48 UTC) #13
commit-bot: I haz the power
4 years, 3 months ago (2016-09-01 06:27:17 UTC) #15
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/83e14103203f581b9b741d2b0612374e88855625
Cr-Commit-Position: refs/heads/master@{#39063}

Powered by Google App Engine
This is Rietveld 408576698