On Fri, Sep 24, 2010 at 7:55 PM, <[email protected]> wrote: > LGTM, but do we have enough test coverage? test/mjsunit/abs.js is somewhat > small, for example it apparently doesn't test the case of the smallest Smi.
Oops, forgot to attach my new test. Please take another look in a minute. -- Vitaly > http://codereview.chromium.org/3446024/diff/1/2 > File src/ia32/macro-assembler-ia32.cc (right): > > http://codereview.chromium.org/3446024/diff/1/2#newcode1578 > src/ia32/macro-assembler-ia32.cc:1578: psllq(dst, 2 * kBitsPerInt - 1); > I wonder if 2 * kBitsPerInt - 1 is a proper way to encode 63 here :) > but up to you > > http://codereview.chromium.org/3446024/diff/1/4 > File src/ia32/stub-cache-ia32.cc (right): > > http://codereview.chromium.org/3446024/diff/1/4#newcode1967 > src/ia32/stub-cache-ia32.cc:1967: if (!object->IsJSObject() || argc != > 1) return Heap::undefined_value(); > pretty minor thing: maybe you shouldn't bail out if argc != 1 and only > process first argument? That's apparently what the spec requires > > http://codereview.chromium.org/3446024/show > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
