LGTM. Some small comments that apply to both platforms.
http://codereview.chromium.org/335005/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/335005/diff/1/2#newcode6541 Line 6541: __ mov(left_arg, left); I like the change with the symbolic names. Maybe this site deserves a comment that the order of moves is important to avoid destroying right_arg/left? http://codereview.chromium.org/335005/diff/1/2#newcode6549 Line 6549: __ mov(right_arg, right); Order of moves also important here. http://codereview.chromium.org/335005/diff/1/2#newcode6555 Line 6555: __ mov(left_arg, left); Here, maybe a comment about what we know: neither left nor right is left_arg or right_arg, and so the order of moves is not important. http://codereview.chromium.org/335005/diff/1/2#newcode6614 Line 6614: bool is_commutative = (op_ == (Token::ADD) || (op_ == Token::MUL)); IsOperationCommutative()? http://codereview.chromium.org/335005/diff/1/2#newcode6618 Line 6618: __ mov(right_arg, Immediate(left)); Do you want SetArgsReversed here? http://codereview.chromium.org/335005/diff/1/5 File src/x64/codegen-x64.h (right): http://codereview.chromium.org/335005/diff/1/5#newcode638 Line 638: // Flag that indicates whether how to generate code for the stub. Extra 'whether'. http://codereview.chromium.org/335005 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
