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

Issue 640603003: [turbofan] Properly emit bounds checks for typed array element loads. (Closed)

Created:
6 years, 2 months ago by Benedikt Meurer
Modified:
6 years, 2 months ago
Reviewers:
Michael Starzinger
CC:
v8-dev
Project:
v8
Visibility:
Public.

Description

[turbofan] Properly emit bounds checks for typed array element loads. Also fix an awfull bug in simplified lowering. TEST=cctest,mjsunit/asm R=mstarzinger@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=24463

Patch Set 1 #

Total comments: 3

Patch Set 2 : Add another test case. #

Patch Set 3 : Fix simplified lowering bugs. #

Patch Set 4 : Add another test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -11 lines) Patch
M src/compiler/js-typed-lowering.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/compiler/representation-change.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/compiler/simplified-lowering.h View 1 2 1 chunk +5 lines, -3 lines 0 comments Download
M src/compiler/simplified-lowering.cc View 1 2 3 chunks +75 lines, -6 lines 0 comments Download
M test/cctest/compiler/test-representation-change.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A test/mjsunit/asm/float32array-outofbounds.js View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
A test/mjsunit/asm/float64array-outofbounds.js View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
A test/mjsunit/asm/int16array-outofbounds.js View 1 2 3 1 chunk +30 lines, -0 lines 0 comments Download
A test/mjsunit/asm/int32array-outofbounds.js View 1 chunk +30 lines, -0 lines 0 comments Download
A test/mjsunit/asm/uint8array-outofbounds.js View 1 2 1 chunk +30 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
Benedikt Meurer
PTAL
6 years, 2 months ago (2014-10-08 09:13:11 UTC) #2
Michael Starzinger
LGTM (just one comment to address). https://codereview.chromium.org/640603003/diff/1/src/compiler/simplified-lowering.cc File src/compiler/simplified-lowering.cc (right): https://codereview.chromium.org/640603003/diff/1/src/compiler/simplified-lowering.cc#newcode876 src/compiler/simplified-lowering.cc:876: void SimplifiedLowering::DoLoadElement(Node* node, ...
6 years, 2 months ago (2014-10-08 09:25:31 UTC) #3
Benedikt Meurer
https://codereview.chromium.org/640603003/diff/1/src/compiler/simplified-lowering.cc File src/compiler/simplified-lowering.cc (right): https://codereview.chromium.org/640603003/diff/1/src/compiler/simplified-lowering.cc#newcode876 src/compiler/simplified-lowering.cc:876: void SimplifiedLowering::DoLoadElement(Node* node, MachineType output_type) { As discussed offline: ...
6 years, 2 months ago (2014-10-08 10:54:58 UTC) #4
Benedikt Meurer
6 years, 2 months ago (2014-10-08 11:17:02 UTC) #5
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as 24463 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698