https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.cc
File src/compiler/interpreter-assembler.cc (right):
https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.cc#newcode137
src/compiler/interpreter-assembler.cc:137: Node*
InterpreterAssembler::BytecodeOperandImm8(int delta) {
On 2015/08/17 10:49:02, oth wrote:
Suggest renaming delta for clarity - operand_index/operand_number.
Done.
https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.h
File src/compiler/interpreter-assembler.h (right):
https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.h#newcode5
src/compiler/interpreter-assembler.h:5: #ifndef
V8_COMPILER_INTERPRETER_CODEGEN_H_
On 2015/08/17 10:49:02, oth wrote:
Aaron Gray correctly highlighted the need for:
s/INTERPRETER_CODEGEN/INTERPRETER_ASSEMBLER/
Done.
https://codereview.chromium.org/1294793002/diff/20001/src/compiler/interpreter-assembler.h#newcode39
src/compiler/interpreter-assembler.h:39: // Returns the Imm8 immediate
for bytecode operand |index| in the current
On 2015/08/17 08:22:34, Michael Starzinger wrote:
nit: Comment seems to be outdated, no index is passed in.
Changed parameter and comment to operand_index
https://codereview.chromium.org/1294793002/diff/20001/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):
https://codereview.chromium.org/1294793002/diff/20001/src/interpreter/interpreter.cc#newcode149
src/interpreter/interpreter.cc:149: __ StoreRegister(__
GetAccumulator(), __ BytecodeOperandReg(0));
On 2015/08/17 08:22:34, Michael Starzinger wrote:
This pattern looks dangerous, note that C++ does not specify the
evaluation
order or the argument, so here either "GetAccumulator" or
"BytecodeOperandReg"
could be assembled first into the bytecode stream.
Yeah good point. Note, in this case it doesn't matter for correctness
which argument is evaluated first, but agree that this could be
confusing and potentially dangerous later. Fixed to define the order.
https://codereview.chromium.org/1294793002/
--
--
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.
