Apart from the below comments LGTM. -Ivan
http://codereview.chromium.org/7302/diff/1/2 File src/codegen-arm.cc (right): http://codereview.chromium.org/7302/diff/1/2#newcode1671 Line 1671: __ ldr(r1, frame_->Element(kNextOffset / kPointerSize)); // read next_sp It would be less error prone if the division by kPointerSize would be done once at the local variable initialization. Maybe you want to rename the local to not be a Offset because that does imply byte offsets. http://codereview.chromium.org/7302/diff/1/2#newcode1690 Line 1690: __ ldr(r1, frame_->Element(kNextOffset / kPointerSize)); ditto http://codereview.chromium.org/7302/diff/1/2#newcode1780 Line 1780: __ ldr(r1, frame_->Element(kNextOffset / kPointerSize)); ditto http://codereview.chromium.org/7302/diff/1/3 File src/codegen-arm.h (right): http://codereview.chromium.org/7302/diff/1/3#newcode68 Line 68: // Index -1 corresponds to the receiver. Can you please add a MemOperand Receiver() const { return Parameter(-1); } This will increase the readability tremendously because then you are not left wondering what the -1 parameter is. Same should be done for the ia32 file. Although at this point I do not see a single explicit reference to Parameter(-1) in the CodeGenerator at all. So this might be just a cosmetic nit. http://codereview.chromium.org/7302 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
