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

Issue 1731603002: Allow bitwise operators to convert from intish to int in heap ops. (Closed)

Created:
4 years, 10 months ago by bradnelson
Modified:
4 years, 10 months ago
Reviewers:
titzer, aseemgarg, bradn
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Allow bitwise operators to convert from intish to int in heap ops. We previously supported use of bitwise operations to convert from intish to int, but use of kAsmInt in some places and kAsmIntQ in others prevents this from working with heap accesses. Switch to use kAsmIntQ where appropriate (even though intish_ != 0 in principle captures the superset of these cases), as it's more conservative (and uses types.h better). BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm R=aseemgarg@chromium.org,titzer@chromium.org LOG=N Committed: https://crrev.com/a52967680edeacb45a97b5c0615eeb346c54a355 Cr-Commit-Position: refs/heads/master@{#34233}

Patch Set 1 #

Total comments: 2

Patch Set 2 : fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -7 lines) Patch
M src/typing-asm.cc View 4 chunks +7 lines, -7 lines 0 comments Download
M test/mjsunit/wasm/asm-wasm.js View 1 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
bradnelson
4 years, 10 months ago (2016-02-24 00:31:10 UTC) #1
aseemgarg
lgtm https://codereview.chromium.org/1731603002/diff/1/test/mjsunit/wasm/asm-wasm.js File test/mjsunit/wasm/asm-wasm.js (right): https://codereview.chromium.org/1731603002/diff/1/test/mjsunit/wasm/asm-wasm.js#newcode1528 test/mjsunit/wasm/asm-wasm.js:1528: (function TestAndIntishAndFromHeap() { Better name? TestAndIntAndHeapValue?
4 years, 10 months ago (2016-02-24 02:16:21 UTC) #2
bradn
https://codereview.chromium.org/1731603002/diff/1/test/mjsunit/wasm/asm-wasm.js File test/mjsunit/wasm/asm-wasm.js (right): https://codereview.chromium.org/1731603002/diff/1/test/mjsunit/wasm/asm-wasm.js#newcode1528 test/mjsunit/wasm/asm-wasm.js:1528: (function TestAndIntishAndFromHeap() { On 2016/02/24 02:16:21, aseemgarg wrote: > ...
4 years, 10 months ago (2016-02-24 07:15:20 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1731603002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1731603002/20001
4 years, 10 months ago (2016-02-24 07:15:37 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 10 months ago (2016-02-24 07:35:15 UTC) #8
commit-bot: I haz the power
4 years, 10 months ago (2016-02-24 07:36:46 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/a52967680edeacb45a97b5c0615eeb346c54a355
Cr-Commit-Position: refs/heads/master@{#34233}

Powered by Google App Engine
This is Rietveld 408576698