Thanks! All incorporated. Tests to follow on this CL before committing.
I should have updated the comment on Push/Pop for temporary registers. The
change could be confusing when looking at code. Semantics are more like
Allocate/Free.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecode-array-builder.cc
File src/interpreter/bytecode-array-builder.cc (right):
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecode-array-builder.cc#newcode29
src/interpreter/bytecode-array-builder.cc:29: int frame_size =
register_count * sizeof(intptr_t);
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
replace sizeof(intptr_t) with kPointerSize (otherwise when cross
compiling the
framesize will be wrong in the serialized snapshot).
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecode-array-builder.h
File src/interpreter/bytecode-array-builder.h (right):
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecode-array-builder.h#newcode25
src/interpreter/bytecode-array-builder.h:25: // Set number of locals
required for bytecode.
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
s/for bytecode/by bytecode array/
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecode-array-builder.h#newcode36
src/interpreter/bytecode-array-builder.h:36: //
===========================================================================
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
nit - this type of header format isn't very common in V8. I would just
have:
// Constant loads to accumulator
BytecodeArrayBuilder& LoadLiteral....
Done. It's copied compiler/code-generator.h.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecodes.h
File src/interpreter/bytecodes.h (right):
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/bytecodes.h#newcode28
src/interpreter/bytecodes.h:28: \
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
super nit - remove newline below comment.
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/interpreter.cc#newcode51
src/interpreter/interpreter.cc:51: // LdaZero <dst>
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
Remove <dst>
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/interpreter.cc#newcode53
src/interpreter/interpreter.cc:53: // Load literal '0' into the
destination register.
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
update comment /s/destination/accumulator/ and remove the code
generation (since
it's wrong now).
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/interpreter.cc#newcode61
src/interpreter/interpreter.cc:61: // LdaSmi8 <dst>, <imm8>
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
remove <dst>
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/interpreter/interpreter.cc#newcode63
src/interpreter/interpreter.cc:63: // Load an 8-bit integer literal into
destination register as a Smi.
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
/s/destination/acumulator
Done.
https://codereview.chromium.org/1266713004/diff/40001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1266713004/diff/40001/src/objects.cc#newcode11633
src/objects.cc:11633: case interpreter::OperandType::kNone:
On 2015/07/31 09:56:19, rmcilroy (OOO until 10th Aug) wrote:
We should never hit this case, right? If so, maybe just add
UNREACHABLE() here
(and move to the bottom)
Done.
https://codereview.chromium.org/1266713004/
--
--
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.
