LGTM with comments
http://codereview.chromium.org/1846002/diff/17001/18001 File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/1846002/diff/17001/18001#newcode3332 src/arm/codegen-arm.cc:3332: ASSERT_EQ(NULL, var); How do we know this assert is true? I would like more comments in this function explaining what the stack holds at each point. Perhaps the flags (is_trivial_receiver, starts_initialization_block, ends_initialization_block, is_compound, var->isnull) can be distilled into a set of orthogonal variables and the stack layout explained for each value of each variable. http://codereview.chromium.org/1846002/diff/17001/18001#newcode3361 src/arm/codegen-arm.cc:3361: Load(node->value()); If this is a constant Smi can we use a SmiOperation instead? http://codereview.chromium.org/1846002/diff/17001/18001#newcode3377 src/arm/codegen-arm.cc:3377: frame_->EmitPop(VirtualFrame::scratch0()); Seems a bit dodgy assuming that the scratch registers don't get overwritten by a trivial receiver. If you are satisfied that this is currently the case then lets leave it in, but we need to start tracking off-stack registers soon. http://codereview.chromium.org/1846002/diff/17001/18001#newcode3406 src/arm/codegen-arm.cc:3406: void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) { All the same comments apply to this function as the function above. http://codereview.chromium.org/1846002/diff/17001/18003 File src/arm/virtual-frame-arm.cc (right): http://codereview.chromium.org/1846002/diff/17001/18003#newcode513 src/arm/virtual-frame-arm.cc:513: __ mov(r1, r0); You forget to update the top_of_stack_state_ here and in the next case. http://codereview.chromium.org/1846002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
