LGTM with comments.
http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode2965 src/ia32/code-stubs-ia32.cc:2965: __ j(greater_equal, &generic_runtime); j(equal) seems to be sufficient as ecx cannot be greater than kExponentMask. http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode2968 src/ia32/code-stubs-ia32.cc:2968: __ jmp(&unpack_exponent, Label::kNear); Move the jump before the empty line? http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode2981 src/ia32/code-stubs-ia32.cc:2981: if (exponent_type_ == ON_STACK) { I wonder if copy-pasting and keeping the ON_STACK and TAGGED cases disjoint would be better. http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode2998 src/ia32/code-stubs-ia32.cc:2998: __ extractps(ecx, xmm1, 4 * kBitsPerByte); Since we already use HeapNumber::kExponentMask below, maybe replace 4 by (HeapNumber::kExponentOffset - HeapNumber::kMantissaOffset)? http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode3029 src/ia32/code-stubs-ia32.cc:3029: // xmm3 now has -0.5. xmm4 now has -0.5. http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode3040 src/ia32/code-stubs-ia32.cc:3040: __ extractps(ecx, xmm1, 4 * kBitsPerByte); Redundant instruction. http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode3098 src/ia32/code-stubs-ia32.cc:3098: // xmm0: base as double that is not +/- Infinity or NaN xmm0 -> xmm1 http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode3100 src/ia32/code-stubs-ia32.cc:3100: // Save exponent in base as we need to check if exponent is negative later. base -> ecx in the comment http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode3121 src/ia32/code-stubs-ia32.cc:3121: // base has the original value of the exponent - if the exponent is base -> ecx in the comment http://codereview.chromium.org/8749002/diff/5005/src/ia32/code-stubs-ia32.cc#newcode3138 src/ia32/code-stubs-ia32.cc:3138: // xmm1: result xmm1 -> xmm3 in the comment. http://codereview.chromium.org/8749002/diff/5005/test/mjsunit/math-pow.js File test/mjsunit/math-pow.js (right): http://codereview.chromium.org/8749002/diff/5005/test/mjsunit/math-pow.js#newcode140 test/mjsunit/math-pow.js:140: assertTrue((1*((Math.pow(2,53))-1)*(Math.pow(2,-1074))) === 4.4501477170144023e-308); Lines are too long. http://codereview.chromium.org/8749002/diff/9030/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/8749002/diff/9030/src/ia32/code-stubs-ia32.cc#newcode2998 src/ia32/code-stubs-ia32.cc:2998: UNREACHABLE(); Since you listed all the cases above, the default case can be removed. This will enable static checks by compiler instead of run-time checks by UNREACHABLE(). http://codereview.chromium.org/8749002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
