Drive-by x64 comments.
http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc#newcode2593 src/x64/macro-assembler-x64.cc:2593: Set(result_reg, 0); How about setting result to 0 prior to the compare, and just jump directly to done if below_equal. Should give less jumping. http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc#newcode2596 src/x64/macro-assembler-x64.cc:2596: uint64_t max_clamp = BitCast<uint64_t, double>(255.0); How about not capping at 255 until after you have converted to integer? http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc#newcode2598 src/x64/macro-assembler-x64.cc:2598: movq(temp_xmm_reg, temp_reg); Use movsd here. Movq makes the CPU think the XMM register holds an integer, not a double. There is a latency for switching between interpretations. http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc#newcode2606 src/x64/macro-assembler-x64.cc:2606: roundsd(temp_xmm_reg, input_reg, kRoundToNearest); Roundsd's round-to-nearest rounds to even numbers in case of a tie. The code in the other branch rounds up. Which one is correct? http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc#newcode2610 src/x64/macro-assembler-x64.cc:2610: movq(temp_xmm_reg, temp_reg); Movsd. http://codereview.chromium.org/7014033/diff/4001/src/x64/macro-assembler-x64.cc#newcode2611 src/x64/macro-assembler-x64.cc:2611: addsd(temp_xmm_reg, input_reg); Adding 0.5 and truncating doesn't work perfectly as round-to-nearet. It doesn't round correctly for 0.49999999999999994. Adding 0.5 incorrectly rounds this up to 1, but round to nearest must round down. You can instead first compare to 0.5, which you already have, and if below, just return 0. (There's also a roundoff problem at 2^53-1, but that doesn't matter here, since both the actual and the correct result will be capped to 255 anyway). http://codereview.chromium.org/7014033/diff/4001/test/mjsunit/external-array.js File test/mjsunit/external-array.js (right): http://codereview.chromium.org/7014033/diff/4001/test/mjsunit/external-array.js#newcode155 test/mjsunit/external-array.js:155: Test 0.49999999999999994, please :). And both 1.5 and 2.5. http://codereview.chromium.org/7014033/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
