LGTM.  I think we could remove a test and a branch from the fast case,
but feel free to do that later.


http://codereview.chromium.org/9040/diff/1/3
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/9040/diff/1/3#newcode1647
Line 1647: __ test(eax, Immediate(0x80000000 | kSmiTag));  // negative
or not Smi
Should probably still be kSmiTagMask.

http://codereview.chromium.org/9040/diff/1/4
File src/runtime.cc (right):

http://codereview.chromium.org/9040/diff/1/4#newcode2564
Line 2564: if (obj -> IsSmi()) {
Please remove spaces around the ->

http://codereview.chromium.org/9040/diff/1/6
File test/mjsunit/regress/regress-137.js (right):

http://codereview.chromium.org/9040/diff/1/6#newcode44
Line 44: throw "Failure: base " + base + " not recognized as Smi in
range 10..15";
Please do an assert false thing here instead.

http://codereview.chromium.org/9040

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to