Thanks! The patch is now submitted.
-- Vitaly http://codereview.chromium.org/3327022/diff/3001/4004 File src/ia32/disasm-ia32.cc (right): http://codereview.chromium.org/3327022/diff/3001/4004#newcode1163 src/ia32/disasm-ia32.cc:1163: int8_t imm8 = *reinterpret_cast<int8_t*>(data + 1); On 2010/09/20 11:56:50, Erik Corry wrote:
Why not just: int imm8 = data[1]; ?
Almost done. I left static_cast there to avoid unsigned to signed warnings. http://codereview.chromium.org/3327022/diff/3001/4007 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/3327022/diff/3001/4007#newcode1871 src/ia32/stub-cache-ia32.cc:1871: // discards NaN. On 2010/09/20 11:56:50, Erik Corry wrote:
It might be nice to detect +- zero and pass it through unchanged.
Zero is
probably reasonably common. Once you have done that then negative
numbers are
perhaps not so hard? It seems like it's the same except you want to
subtract
then add instead of adding then subtracting below and then you need to
reverse
the rest too. You can save this for another change or not do it at
all :-) I filed a bug to consider extending it. http://codereview.chromium.org/3327022/diff/3001/4010 File test/mjsunit/math-floor.js (right): http://codereview.chromium.org/3327022/diff/3001/4010#newcode31 test/mjsunit/math-floor.js:31: assertEquals(0, Math.floor(0)); On 2010/09/20 11:56:50, Erik Corry wrote:
This tests the Smi 0 but not the heap number 0. Normally + will not
check for a
Smi result so this should work:
x = 0.5 y = 0.5 assertEquals(x - y, Math.floor(x - y)))
Done. I left the smi test case for completeness. http://codereview.chromium.org/3327022/diff/3001/4010#newcode32 test/mjsunit/math-floor.js:32: assertEquals(-0, Math.floor(-0)); On 2010/09/20 11:56:50, Erik Corry wrote:
0 == -0 so this test doesn't really work. You need to do
1/-0, 1/Math.floor(-0)
Good catch! Fixed. http://codereview.chromium.org/3327022/diff/3001/4010#newcode60 test/mjsunit/math-floor.js:60: var two_30 = 1 << 30; On 2010/09/20 11:56:50, Erik Corry wrote:
It would be nice to pass this in as an argument so you could easily do
two_53 as
well.
There are actually some differences. But even if there were none, I still think having verbose test code would be better: it's much easier to spot an error. http://codereview.chromium.org/3327022/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
