The Use/Unuse protocol in AdjustCopies should be simplified, then it LGTM.
I have a couple of other comments that could be saved for another change. I think we still need more aggressive refactoring---there are too many places where we end up doing almost the same thing (adjusting copies, unusing registers, overwriting frame element). http://codereview.chromium.org/50012/diff/1/2 File src/virtual-frame-ia32.cc (right): http://codereview.chromium.org/50012/diff/1/2#newcode525 Line 525: __ mov(backing_reg, Operand(ebp, fp_relative(index))); I think it's clearer to Use(backing_reg) just before or after this line rather than unusing it in the register case and then calling use on it again in both cases. That way, there is a single use on the memory case, and no unuse/use on the register case. http://codereview.chromium.org/50012/diff/1/2#newcode535 Line 535: new_backing_element.set_sync(); You might consider just putting the creation of the new element and assignment to elements_[i] in this conditional (making it an if/else). If we're going to test the sync flag, we might as well create it with the right one in the first place. http://codereview.chromium.org/50012/diff/1/3 File src/virtual-frame.cc (right): http://codereview.chromium.org/50012/diff/1/3#newcode388 Line 388: Unuse(elements_[frame_index].reg()); It seems like we actually want to make (a suitably renamed) "AdjustCopies" always invalidate the frame element at the argument index (unusing the register), and we might be able to simplify this so we don't need the second test? I don't think we ever need the frame slot after AdjustCopies (that's the point), so call it "InvalidateFrameSlotAt" or something. It's a little weird to emit code for a frame element marked invalid, so we could go farther and have SetFrameSlotAt and pass it in invalid element in the one case where we *actually* want it to be invalidated. What do you think? http://codereview.chromium.org/50012 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
