LGTM.
http://codereview.chromium.org/113400/diff/1/2 File src/ia32/jump-target-ia32.cc (right): http://codereview.chromium.org/113400/diff/1/2#newcode228 Line 228: reaching_frames_.length() == 1) { Can't this case be reduced to the previous case (1 reaching frame, no linked frames) using the code below, for both cases, forward and backward links? Then the lines below making cgen->frame frames_[0], and dropping frames_[0], could be before line 198, and reuse that code. Would that be better? http://codereview.chromium.org/113400/diff/1/3 File src/ia32/virtual-frame-ia32.cc (right): http://codereview.chromium.org/113400/diff/1/3#newcode194 Line 194: elements_[i].clear_copied(); On 2009/05/15 06:26:51, Kasper Lund wrote: > Can't you add support for coping the is_copied flag directly? All this clearing, > testing, and setting seems complicated. I recall that when we tried any sort of list or count of the number of copies, so that we could accurately know when the last copy was removed, performance decreased. http://codereview.chromium.org/113400/diff/1/3#newcode213 Line 213: Result fresh = cgen_->allocator()->Allocate(); If there is a preference to allocate slots at the top of the frame to registers, this accomplishes that. Maybe a comment saying "put copies and constants in registers, working upward to the top of the frame" http://codereview.chromium.org/113400/diff/1/4 File src/ia32/virtual-frame-ia32.h (right): http://codereview.chromium.org/113400/diff/1/4#newcode118 Line 118: How about: // Remove copies and constants from the top mergeable_elements of the frame, // so that a frame of the same height that differs only on these elements can be merged to it. // Use JumpTarget::kAllElements if the entire frame should be made mergeable. http://codereview.chromium.org/113400 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
