This LGTM, but I think you should get rid of the drop/push sequence in a couple of places and clean up some comments.
http://codereview.chromium.org/19622/diff/1/2 File src/codegen-ia32.cc (left): http://codereview.chromium.org/19622/diff/1/2#oldcode3591 Line 3591: // We need the CC bits to come out as not_equal in the case where the It's nice to get rid of this. http://codereview.chromium.org/19622/diff/1/2 File src/codegen-ia32.cc (right): http://codereview.chromium.org/19622/diff/1/2#newcode3604 Line 3604: Result temp = allocator()->Allocate(); We will need to sort out the decision to spill an existing register (eg, value) or allocate a fresh one. A simple solution is if the existing register is unaliased in the frame, reuse it, otherwise allocate. A more sophisticated solution considers the cost of spilling vs. allocating. Allocating will never consider the actually unneeded off-frame register, because it thinks it is needed. http://codereview.chromium.org/19622/diff/1/2#newcode3620 Line 3620: // Seed the result with the formal parameters count, which will be I would reword this comment to be more explicit. The real point is that the (read length version of the) arguments access stub takes the parameter count as an argument passed in eax. http://codereview.chromium.org/19622/diff/1/2#newcode3657 Line 3657: frame_->Push(&temp); Can we avoid the drop/push sequence here by setting the top element? http://codereview.chromium.org/19622/diff/1/2#newcode3705 Line 3705: frame_->Drop(); Can we avoid the drop/push sequence here by setting the top element? http://codereview.chromium.org/19622/diff/1/2#newcode3719 Line 3719: left.ToRegister(); I would rather not move left to a register, but generate a difference cmp instruction in the case it's a constant. http://codereview.chromium.org/19622/diff/1/2#newcode3732 Line 3732: VirtualFrame::SpilledScope spilled_scope(this); Double check that this is scope is gone once you update to include your immediately previous change list. http://codereview.chromium.org/19622 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
