Thanks, have put changes into:
https://codereview.chromium.org/1259193004
https://codereview.chromium.org/1266713004/diff/120001/src/interpreter/bytecode-array-builder.cc
File src/interpreter/bytecode-array-builder.cc (right):
https://codereview.chromium.org/1266713004/diff/120001/src/interpreter/bytecode-array-builder.cc#newcode147
src/interpreter/bytecode-array-builder.cc:147:
DCHECK_EQ(Bytecodes::NumberOfOperands(bytecode), 3);
On 2015/08/03 11:06:01, picksi wrote:
This function does a DCHECK_EQ, the following functions use DCHECK, is
this
intentional?
Fixed, thanks!
https://codereview.chromium.org/1266713004/diff/120001/src/interpreter/bytecode-array-builder.cc#newcode196
src/interpreter/bytecode-array-builder.cc:196: return
static_cast<Bytecode>(-1);
On 2015/08/03 11:06:01, picksi wrote:
Should we have an error ByteCode that is -1, instead of casting it
here?
In this instance, it's not recoverable error. The failure mode of
UNIMPLEMENTED() is a fatal which makes the source easy to find. There
are more operands to come here.
The test for the BytecodeArrayBuilder checks all bytecodes can be
generated by the builder. A function in the builder generating an
invalid bytecode would probably get used.
https://codereview.chromium.org/1266713004/diff/120001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1266713004/diff/120001/src/objects.cc#newcode11614
src/objects.cc:11614: int bytes = 0;
On 2015/08/03 11:06:01, picksi wrote:
Can we name 'bytes' to say what this is (the size of a
bytecode+params, the size
of a function?)? It is not clear!
Reworked in https://codereview.chromium.org/1259193004
https://codereview.chromium.org/1266713004/diff/120001/src/objects.cc#newcode11628
src/objects.cc:11628: for (int j = 1; j < bytes; j++) {
On 2015/08/03 11:06:01, picksi wrote:
Should this for-loop be pulled out into a separate function? It looks
to be
conceptually at a lower level (disassembling a single instruction?).
Also, the value of 'j' is used as j-1, j and j+1. Is there a way to
rework this
to not need some many variations!
Reworked in https://codereview.chromium.org/1259193004.
https://codereview.chromium.org/1266713004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
