These changes finally made, and submitted in changelist 51007. http://codereview.chromium.org/51007/show
http://codereview.chromium.org/42006/diff/1/3 File src/codegen-ia32.cc (right): http://codereview.chromium.org/42006/diff/1/3#newcode686 Line 686: // should be inlined, placed in the stub, or omitted entirely. On 2009/03/10 10:14:47, Kevin Millikin wrote: > To parallel the order of the enum, "should be placed in the stub, inlined, or > omitted entirely". Done. http://codereview.chromium.org/42006/diff/1/3#newcode722 Line 722: class OpBits: public BitField<Token::Value, 2, 12> {}; On 2009/03/10 10:14:47, Kevin Millikin wrote: > You might consider asserting in the constructor that NUM_TOKENS fits in 12 bits. Done. http://codereview.chromium.org/42006/diff/1/3#newcode834 Line 834: // Compute the result, and return that as a constant on the frame. On 2009/03/26 12:55:46, William Hesse wrote: > On 2009/03/10 10:14:47, Kevin Millikin wrote: > > Maybe should avoid words like "return" because they can be ambiguous (return > > from this C++ function? return at runtime?). "Compute the result and leave > it > > on the frame."? > Done. Done. http://codereview.chromium.org/42006/diff/1/3#newcode860 Line 860: // The same stub is used for NO_SMI_CODE and SMI_CODE_INLINED. On 2009/03/10 10:14:47, Kevin Millikin wrote: > It's probably better to make the stub handle all three values of the enum rather > than just two. It's less confusing here and will be probably be easier to > change later by just changing the stub. Changed back to two values of the flags, and a separate boolean flag. http://codereview.chromium.org/42006/diff/1/3#newcode956 Line 956: // Create a new deferred code object that calls GenericBinaryOpStub On 2009/03/10 10:14:47, Kevin Millikin wrote: > It calls a (specialized) instance of a GenericBinaryOpStub, not the > GenericBinaryOpStub. (But I don't think you even need to mention that here, > because this non-local comment becomes out of date if the deferred code > changes). Done. http://codereview.chromium.org/42006/diff/1/3#newcode5620 Line 5620: __ or_(answer.reg(), Operand(right->reg())); On 2009/03/10 10:14:47, Kevin Millikin wrote: > (As a future change, we could simplify the smi check code if left and right are > the same registers, eg, "x + x" or "y = x; x + y".) Done. http://codereview.chromium.org/42006 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
