http://codereview.chromium.org/19425/diff/1/3 File src/codegen-ia32.cc (right):
http://codereview.chromium.org/19425/diff/1/3#newcode3480 Line 3480: set_in_spilled_code(false); On 2009/01/29 10:16:53, Kevin Millikin wrote: > Don't forget to remove this. When I merge with the current version of toiger, I will. Right now, it is needed. Merging with current version of toiger is the next step. http://codereview.chromium.org/19425/diff/1/3#newcode3489 Line 3489: // Load the string into eax and the index into ebx. On 2009/01/29 10:16:53, Kevin Millikin wrote: > Drop this comment. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3495 Line 3495: Result index = frame_->Pop(); // ebx in previous code On 2009/01/29 10:16:53, Kevin Millikin wrote: > Remove this comment before submitting. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3497 Line 3497: Result object = frame_->Pop(); // eax in previous code On 2009/01/29 10:16:53, Kevin Millikin wrote: > And this one too. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3535 Line 3535: object_type.Unuse(); On 2009/01/29 10:16:53, Kevin Millikin wrote: > There's no real reason to unuse object_type here. You could consider just > aliasing it with length, using length where it makes sense, and then unusing > length later. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3554 Line 3554: // Pass shift_amount register (ecx) to this branch so it can be merged back On 2009/01/29 10:16:53, Kevin Millikin wrote: > This comment is a little confusing. shift_amount is called object_type at the > binding of the jump target, and it is live in that block so it was not passed > just to merge it back at the backward edge. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3571 Line 3571: ascii_string.Bind(); On 2009/01/29 10:16:53, Kevin Millikin wrote: > Arguments were passed at ascii_string.Branch, so they should be picked up here > at Bind. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3574 Line 3574: times_1, On 2009/01/29 10:16:53, Kevin Millikin wrote: > Indentation is off. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3578 Line 3578: got_char_code.Bind(&object_type); On 2009/01/29 10:16:53, Kevin Millikin wrote: > This is confusing. The only non-frame value passed from unconditional jump is > object_type, but shift_amount is live after the Bind. This was an error - shift_amount should not have been used. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3585 Line 3585: not_a_flat_string.Bind(&object, &index, &object_type); On 2009/01/29 10:16:53, Kevin Millikin wrote: > A little confusing that there is only one jump to this label, and it passes > shift_reg instead of object_type. This was an error. http://codereview.chromium.org/19425/diff/1/3#newcode3588 Line 3588: a_cons_string.Branch(equal, &object, &index, &shift_amount, not_taken); On 2009/01/29 10:16:53, Kevin Millikin wrote: > You negated the branch but not the hint, for what that's worth. Done. http://codereview.chromium.org/19425/diff/1/3#newcode3612 Line 3612: frame_->EmitPush(Immediate(Factory::undefined_value())); On 2009/01/29 10:16:53, Kevin Millikin wrote: > This should be a regular Push, not a raw one, since the frame is unspilled. Done. http://codereview.chromium.org/19425 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
