LGTM, but I have some style suggestions.
http://codereview.chromium.org/551080/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/551080/diff/1/2#newcode917 src/ia32/codegen-ia32.cc:917: GenericBinaryFlags flags; It looks like the flags aren't needed until later, and aren't always needed. You could consider moving this switch later in the function so the use isn't so far from the definition. We also have a function Token::IsBitOp that helps here: if (loop_nesting() > 0 && (Token::IsBitOp(op) || type->IsLikelySmi())) { flags = NO_SMI_CODE_IN_STUB; } http://codereview.chromium.org/551080/diff/1/2#newcode991 src/ia32/codegen-ia32.cc:991: type, false, overwrite_mode); Indentation is now off here. http://codereview.chromium.org/551080/diff/1/2#newcode993 src/ia32/codegen-ia32.cc:993: answer = ConstantSmiBinaryOperation(op, &right, left.handle(), And here. http://codereview.chromium.org/551080/diff/1/2#newcode1664 src/ia32/codegen-ia32.cc:1664: return LikelySmiBinaryOperation(op, &constant_operand, operand, You could do 'answer =' instead of 'return' here for symmetry with the other cases. http://codereview.chromium.org/551080/diff/1/2#newcode1692 src/ia32/codegen-ia32.cc:1692: return LikelySmiBinaryOperation(op, &constant_operand, operand, Same comment. http://codereview.chromium.org/551080/diff/1/2#newcode1874 src/ia32/codegen-ia32.cc:1874: return LikelySmiBinaryOperation(op, &constant_operand, operand, Maybe have 'answer =' here instead of 'return'. http://codereview.chromium.org/551080/diff/1/2#newcode1877 src/ia32/codegen-ia32.cc:1877: return LikelySmiBinaryOperation(op, operand, &constant_operand, And here. http://codereview.chromium.org/551080
-- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
