http://codereview.chromium.org/99052/diff/1/2 File src/ia32/virtual-frame-ia32.cc (right):
http://codereview.chromium.org/99052/diff/1/2#newcode315 Line 315: if (register_locations_[i] == expected->register_locations_[i]) continue; register_locations_[i] and register_index(from) are used multiple times in the loop body and don't change. It's clearer to name them with a local variable. http://codereview.chromium.org/99052/diff/1/2#newcode317 Line 317: && expected->elements_[register_locations_[i]].is_register()) { Is it the case that if is_used(i) then expected->elements_[register_locations_[i]] must be a register? We have already done register-to-memory and register-to-invalid moves, and I don't think anything else is possible. http://codereview.chromium.org/99052/diff/1/2#newcode320 Line 320: if (is_used(to) && !to.is(from)) { Doesn't register_locations_[i] != expected->register_locations_[i] (and the property that registers are singly referenced in the frame) imply !to.is(from)? Maybe that should just be changed to an assert if you're worried about it. http://codereview.chromium.org/99052/diff/1/2#newcode328 Line 328: // Terrible thing to do, but necessary: I agree it's terrible, but is it really necessary? What's really going on is that there are nested loops---the outer one iterates over all registers in the current frame, and the inner one is a while loop iterating until a register reaches its proper frame index. It's a little confusing to share the same loop for both purposes. It might even eliminate some work (like the test is_used(i) and initializing from) to add an explicit inner loop. http://codereview.chromium.org/99052/diff/1/2#newcode330 Line 330: } else if (!to.is(from)) { Is it possible for to.is(from)? http://codereview.chromium.org/99052/diff/1/2#newcode350 Line 350: int j = expected->register_locations_[i]; Since j is not a loop index, choose a descriptive name (eg, frame_index or something). http://codereview.chromium.org/99052 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
