Looking better, but still some comments. It will be great to see it in place because it blocks most loop bodies.
I think you lost the automatic Unuse of the register in the destructor for is_register() and !is_no(reg) results---make sure that gets in in this change or the next one if its needed. http://codereview.chromium.org/13665/diff/15/16 File src/codegen-ia32.cc (right): http://codereview.chromium.org/13665/diff/15/16#newcode1205 Line 1205: VirtualFrame* frame_; The frame is not needed here---the superclass will handle it. http://codereview.chromium.org/13665/diff/15/16#newcode1218 Line 1218: // TODO Add in entry and exit frames for deferred code. This should already be plumbed in through the superclass and the code generator. Format todos on the branch as "TODO(): " for now, unless we file an issue. http://codereview.chromium.org/13665/diff/15/16#newcode1235 Line 1235: Register comparee_reg = allocator()->Allocate(); I still think this whole operation will be common enough to have its own abstraction, something like Result::ToRegister. http://codereview.chromium.org/13665/diff/15/16#newcode3998 Line 3998: frame_->SpillAll(); This should be safe to remove---the call to Load will spill if it needs to. http://codereview.chromium.org/13665/diff/15/16#newcode4146 Line 4146: frame_->SpillAll(); Could be removed. http://codereview.chromium.org/13665/diff/15/16#newcode4156 Line 4156: frame_->SpillAll(); Ditto. http://codereview.chromium.org/13665/diff/15/16#newcode4174 Line 4174: frame_->SpillAll(); I think everything will work if we remove the SpillAll here and after the call to load, since SmiComparison is fixed up. http://codereview.chromium.org/13665/diff/15/16#newcode4186 Line 4186: frame_->SpillAll(); You could also remove this one. http://codereview.chromium.org/13665 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
