The register shuffling code looks good. I'm not sure if there's any good way to abstract it or if we just handle it on a case-by-case basis. We'll see as we hit more cases like this.
http://codereview.chromium.org/21538/diff/2001/2003 File src/codegen-ia32.cc (right): http://codereview.chromium.org/21538/diff/2001/2003#newcode753 Line 753: Result GenerateInlineCode(Result* left, Result* right); This needs a comment both here and at the implementation telling whether the results "left" and "right" are consumed. http://codereview.chromium.org/21538/diff/2001/2003#newcode833 Line 833: if (!right_const_smi) { I'd rather not see negated conditions when there is both a then and else part. It seems more straightforward to swap them. http://codereview.chromium.org/21538/diff/2001/2003#newcode929 Line 929: if (answer_object == Heap::undefined_value_) { Use the accessor Heap::undefined_value(). http://codereview.chromium.org/21538/diff/2001/2003#newcode932 Line 932: Result answer_result(Handle<Object *>(answer_object), this); You mean Handle<Object>? No space between "Object" and "*". Don't we have a VirtualFrame::Push member function that takes a handle anyway, so there's no need to build an result. I think that explicitly constructing a Result object should be treated as a bad smell. http://codereview.chromium.org/21538/diff/2001/2003#newcode5438 Line 5438: __ mov(answer.reg(), Operand(eax)); Use the register-register encoding of mov, not the register-operand one. http://codereview.chromium.org/21538/diff/2001/2003#newcode5467 Line 5467: __ mov(answer.reg(), Operand(edx)); Ditto. http://codereview.chromium.org/21538/diff/2001/2003#newcode5477 Line 5477: __ mov(eax, Operand(left.reg())); Ditto. http://codereview.chromium.org/21538/diff/2001/2002 File src/codegen-ia32.h (right): http://codereview.chromium.org/21538/diff/2001/2002#newcode483 Line 483: void SmiOperation(Result* operand, This needs a comment in the header telling whether it consumes the result "operand" or not. http://codereview.chromium.org/21538 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
