A round of comments. Also make sure it lints.
http://codereview.chromium.org/7132002/diff/19001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): http://codereview.chromium.org/7132002/diff/19001/src/hydrogen-instructions.h#newcode2501 src/hydrogen-instructions.h:2501: if (index == 0) { I sort of prefer return (index == 0) ? Representation::Tagged() : representation(); but I guess you don't? http://codereview.chromium.org/7132002/diff/19001/src/hydrogen-instructions.h#newcode2644 src/hydrogen-instructions.h:2644: HValue* left() { return value(); } It might make sense to define left() before right() here :) http://codereview.chromium.org/7132002/diff/19001/src/hydrogen-instructions.h#newcode3908 src/hydrogen-instructions.h:3908: return Representation::Integer32(); Context should be tagged. http://codereview.chromium.org/7132002/diff/19001/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): http://codereview.chromium.org/7132002/diff/19001/src/ia32/lithium-codegen-ia32.cc#newcode3543 src/ia32/lithium-codegen-ia32.cc:3543: __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset)); This needs a comment that we're potentially passing the wrong context (in the case of an inlined function). http://codereview.chromium.org/7132002/diff/19001/src/ia32/lithium-codegen-ia32.h File src/ia32/lithium-codegen-ia32.h (left): http://codereview.chromium.org/7132002/diff/19001/src/ia32/lithium-codegen-ia32.h#oldcode182 src/ia32/lithium-codegen-ia32.h:182: ContextMode context_mode); Yay! http://codereview.chromium.org/7132002/diff/19001/src/ia32/lithium-ia32.cc File src/ia32/lithium-ia32.cc (right): http://codereview.chromium.org/7132002/diff/19001/src/ia32/lithium-ia32.cc#newcode2167 src/ia32/lithium-ia32.cc:2167: new LDeleteProperty(UseFixed(instr->context(), esi), I like to name these UseXXX subexpressions and linearize them. I'm not sure if we still have an evaluation-order dependency, but I'd like the register allocator to be determinstic wrt. the compiler's evaluation order. So feel free to clean them up as you notice them. http://codereview.chromium.org/7132002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
