LGTM, with comments below.

http://codereview.chromium.org/6461021/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6461021/diff/1/src/hydrogen-instructions.h#newcode1368
src/hydrogen-instructions.h:1368: if (!to.IsTagged()) {
This function always makes me nervous, because the developer has to
understand that it's not OK to change from an untagged representation
back to tagged.

Maybe we should handle the else case here, or assert that if
to.IsTagged() then from.IsTagged().

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode1027
src/ia32/lithium-codegen-ia32.cc:1027: __ test(eax, Operand(eax));
eax ==> input?

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode1042
src/ia32/lithium-codegen-ia32.cc:1042: __ psllq(xmm0, 32);
Gesundheit.

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-ia32.h#newcode1059
src/ia32/lithium-ia32.h:1059: explicit LNegI(LOperand* value) {
I've been trying to create simple accessors for the inputs (e.g.,
LOperand* value() { return inputs_[0]; }).

It's not so bad for unary operations, but for the others I don't want
the programmer to have to remember the position of the arguments.

It also makes it easier to add arguments or reorder them in the
LInstruction without having to guard against explicit use of indices.

http://codereview.chromium.org/6461021/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to