http://codereview.chromium.org/13665/diff/215/216 File src/codegen-ia32.cc (right):
http://codereview.chromium.org/13665/diff/215/216#newcode1233 Line 1233: // TODO Could be optimized to a jump at compile time. TODOs should have a bug number associated with it. I know this is on a branch and if you intend to fix it before work on this branch is merged to bleeding_edge, then it is OK to leave it like this. http://codereview.chromium.org/13665/diff/215/218 File src/virtual-frame-ia32.h (right): http://codereview.chromium.org/13665/diff/215/218#newcode75 Line 75: void ConvertConstantToRegister(); How about just calling this function Load() and that function should do the right thing based on this item's type? Then you could use it as follows: Item comparee = frame_->Pop(); comparee.Load(); ASSERT(comparee.is_register()); // This ASSERT is unneeded as comparee.reg() does make sure that reg() is not called on items that are not of register type. It is left here for demonstration. __ test(comparee.reg(), Immediate(kSmiTagMask)); If you don't do it this way you will distribute the checks all over the code generator and they will keep growing when we eventually add memory based items. Overall this will lead to toit'er code. http://codereview.chromium.org/13665/diff/215/218#newcode77 Line 77: enum Type {REGISTER, CONSTANT, INVALID}; In general we list different enum values on separate lines. I know the code generator is the biggest offender of this rule, but that does not mean we should propagate that behaviour. http://codereview.chromium.org/13665 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
