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

Issue 1259193004: [Interpreter] Consistency fixes. (Closed)

Created:
5 years, 4 months ago by oth
Modified:
5 years, 4 months ago
Reviewers:
Michael Starzinger
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[Interpreter] Consistency fixes. Change minimum BytecodeArray frame size to zero now return value is in the accumulator. Fix inconsistent checks in bytecode-array-builder.cc. Simplify bytecode disassembly by adding Bytecodes::Decode to disassemble one bytecode and operands. BUG=v8:4280 LOG=N Committed: https://crrev.com/d689c7a7be90c01d13d87f6cf0281ceae1f5f813 Cr-Commit-Position: refs/heads/master@{#29988}

Patch Set 1 #

Total comments: 1

Patch Set 2 : Remove redundant argument to Bytecodes::Decode(). #

Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -41 lines) Patch
M src/interpreter/bytecode-array-builder.cc View 4 chunks +4 lines, -3 lines 0 comments Download
M src/interpreter/bytecodes.h View 1 1 chunk +5 lines, -2 lines 0 comments Download
M src/interpreter/bytecodes.cc View 1 1 chunk +42 lines, -0 lines 0 comments Download
M src/objects.cc View 1 1 chunk +12 lines, -33 lines 0 comments Download
M src/objects-inl.h View 1 chunk +1 line, -2 lines 0 comments Download
M test/cctest/interpreter/test-bytecode-array-builder.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 18 (7 generated)
oth
mstarzinger PTAL - this is minor clean-up. picksi - this change attempts to incorporate your ...
5 years, 4 months ago (2015-08-03 15:45:42 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1259193004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1259193004/1
5 years, 4 months ago (2015-08-03 15:46:25 UTC) #4
commit-bot: I haz the power
Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even ...
5 years, 4 months ago (2015-08-03 15:46:27 UTC) #6
Michael Starzinger
LGTM with one optional suggestion. https://codereview.chromium.org/1259193004/diff/1/src/interpreter/bytecodes.h File src/interpreter/bytecodes.h (right): https://codereview.chromium.org/1259193004/diff/1/src/interpreter/bytecodes.h#newcode104 src/interpreter/bytecodes.h:104: const uint8_t* bytecode_end); It ...
5 years, 4 months ago (2015-08-03 15:59:09 UTC) #7
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1259193004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1259193004/20001
5 years, 4 months ago (2015-08-03 16:16:28 UTC) #9
oth
On 2015/08/03 15:59:09, Michael Starzinger wrote: > LGTM with one optional suggestion. > > https://codereview.chromium.org/1259193004/diff/1/src/interpreter/bytecodes.h ...
5 years, 4 months ago (2015-08-03 16:17:21 UTC) #10
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-03 16:41:32 UTC) #12
picksi
Thanks! These changes work well.
5 years, 4 months ago (2015-08-03 17:13:32 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1259193004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1259193004/20001
5 years, 4 months ago (2015-08-03 20:36:59 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 4 months ago (2015-08-03 20:39:23 UTC) #17
commit-bot: I haz the power
5 years, 4 months ago (2015-08-03 20:39:42 UTC) #18
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/d689c7a7be90c01d13d87f6cf0281ceae1f5f813
Cr-Commit-Position: refs/heads/master@{#29988}

Powered by Google App Engine
This is Rietveld 408576698