I know this isn't completed or tested, but I found some possible sources of bugs.
As for your specific questions, I really don't know. It seems like we should probably get it working, and then experiment with various versions on benchmarks where it gets exercised. 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); Don't forget to remove this. http://codereview.chromium.org/19425/diff/1/3#newcode3489 Line 3489: // Load the string into eax and the index into ebx. Drop this comment. http://codereview.chromium.org/19425/diff/1/3#newcode3495 Line 3495: Result index = frame_->Pop(); // ebx in previous code Remove this comment before submitting. http://codereview.chromium.org/19425/diff/1/3#newcode3497 Line 3497: Result object = frame_->Pop(); // eax in previous code And this one too. http://codereview.chromium.org/19425/diff/1/3#newcode3535 Line 3535: object_type.Unuse(); 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. 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 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. http://codereview.chromium.org/19425/diff/1/3#newcode3571 Line 3571: ascii_string.Bind(); Arguments were passed at ascii_string.Branch, so they should be picked up here at Bind. http://codereview.chromium.org/19425/diff/1/3#newcode3574 Line 3574: times_1, Indentation is off. http://codereview.chromium.org/19425/diff/1/3#newcode3578 Line 3578: got_char_code.Bind(&object_type); This is confusing. The only non-frame value passed from unconditional jump is object_type, but shift_amount is live after the Bind. http://codereview.chromium.org/19425/diff/1/3#newcode3585 Line 3585: not_a_flat_string.Bind(&object, &index, &object_type); A little confusing that there is only one jump to this label, and it passes shift_reg instead of object_type. http://codereview.chromium.org/19425/diff/1/3#newcode3588 Line 3588: a_cons_string.Branch(equal, &object, &index, &shift_amount, not_taken); You negated the branch but not the hint, for what that's worth. http://codereview.chromium.org/19425/diff/1/3#newcode3612 Line 3612: frame_->EmitPush(Immediate(Factory::undefined_value())); This should be a regular Push, not a raw one, since the frame is unspilled. http://codereview.chromium.org/19425 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
