Thanks, incorporated.
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc
File src/compiler/bytecode-graph-builder.cc (right):
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc#newcode35
src/compiler/bytecode-graph-builder.cc:35: //
On 2015/09/04 11:35:37, rmcilroy wrote:
nit - drop extra "//" here and at end of comment
Done.
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc#newcode36
src/compiler/bytecode-graph-builder.cc:36: // values_ layout
On 2015/09/04 11:35:37, rmcilroy wrote:
nit - /s/values_ layout/The layout of values_ is:/
Done.
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc#newcode45
src/compiler/bytecode-graph-builder.cc:45: // receiver
On 2015/09/04 11:35:37, rmcilroy wrote:
nit - /s/receiver/Reciever./ (and similar below)
Done.
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc#newcode71
src/compiler/bytecode-graph-builder.cc:71: // values layout is
[receiver] [parameters] [registers]
On 2015/09/04 11:35:37, rmcilroy wrote:
nit - remove this comment (it will probably end up getting out of sync
with the
same comment in Environment()).
Done.
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/bytecode-graph-builder.cc#newcode285
src/compiler/bytecode-graph-builder.cc:285: BuildBinaryOp(node);
On 2015/09/04 11:35:37, rmcilroy wrote:
I was meaning that the BuildBinaryOp would take js_op, left, right and
build the
node, rather than just finishing the node once it's done (i.e., move
the Node*
node = NewNode(js_op, left, right) in each VisitAdd/Sub... into the
BuildBinaryOp.
Done.
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/linkage.h
File src/compiler/linkage.h (right):
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/linkage.h#newcode333
src/compiler/linkage.h:333: static const int
kInterpreterReceiverParameter = -1;
On 2015/09/04 11:35:37, rmcilroy wrote:
No, please don't make this an Interpreter parameter - these are only
the
parameters which are passed via the Dispatch TailCalls in the bytecode
handler.
It should probably be just below kJSFunctionCallClosureParamIndex and
be called
something like kJSFunctionRecieverParamIndex (since this is not
specific to the
interpreter).
Actually, I'm wondering why this isn't
Linkage::kJSFunctionCallClosureParamIndex - this seems to be what
ASTGraphBuilder is using for the same thing. Michi, why didn't you
want this to
be Linkage::kJSFunctionCallClosureParamIndex?
Removed. Won't commit until this is resolved. Comment added to
BytecodeGraphBuilder next to -1.
https://codereview.chromium.org/1291693004/diff/160001/src/interpreter/bytecode-array-iterator.cc
File src/interpreter/bytecode-array-iterator.cc (right):
https://codereview.chromium.org/1291693004/diff/160001/src/interpreter/bytecode-array-iterator.cc#newcode15
src/interpreter/bytecode-array-iterator.cc:15: :
bytecode_array_(bytecode_array), bytecode_offset_(0) {
On 2015/09/04 11:35:37, rmcilroy wrote:
need to init operands_used_ if DEBUG
Yes, but that's what MarkOperandsUnused() does.
https://codereview.chromium.org/1291693004/diff/160001/src/interpreter/bytecode-array-iterator.cc#newcode31
src/interpreter/bytecode-array-iterator.cc:31: bool
BytecodeArrayIterator::More() const {
On 2015/09/04 11:35:37, rmcilroy wrote:
More seems a little confusing here, since it implies you could run
until More()
and still call current_bytecode() even if More != true (since it
implies that
you can't call Next, not that we have gone past the current value).
How about
done() (like StackFrameIteratorBase)? (also maybe Advance() instead of
Next()
also like StackFrameIteratorBase?)
Done. These were adopted from elsewhere in the tree.
https://codereview.chromium.org/1291693004/diff/160001/src/interpreter/bytecode-array-iterator.cc#newcode86
src/interpreter/bytecode-array-iterator.cc:86: void
BytecodeArrayIterator::CheckOperandsUsed() const {
On 2015/09/04 11:35:37, rmcilroy wrote:
I'm wondering how useful this will be (and whether we will end up
needing to
iterate over bytecode and not always check all the bytecode operands).
I'm fine
with having it here though if you think it would be useful.
Gone :-)
https://codereview.chromium.org/1291693004/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
